The -fstrict-flex-arrays=3 option is now available with the release
of GCC 13[1] and Clang 16[2]. This feature instructs the compiler to
treat only C99 flexible arrays as dynamically sized for the purposes of
object size calculations. In other words, the ancient practice of using
1-element arrays, or the GNU extension of using 0-sized arrays, as a
dynamically sized array is disabled. This allows CONFIG_UBSAN_BOUNDS,
CONFIG_FORTIFY_SOURCE, and other object-size aware features to behave
unambiguously in the face of trailing arrays: only C99 flexible arrays
are considered to be dynamically sized.
Enabling this will help track down any outstanding cases of fake
flexible arrays that need attention in kernel code.
[1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays
[2] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fstrict-flex-arrays
Cc: Masahiro Yamada <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Nicolas Schier <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Makefile b/Makefile
index f836936fb4d8..07e5aec1daf5 100644
--- a/Makefile
+++ b/Makefile
@@ -1026,6 +1026,12 @@ KBUILD_CFLAGS += -Wno-pointer-sign
# globally built with -Wcast-function-type.
KBUILD_CFLAGS += $(call cc-option, -Wcast-function-type)
+# To gain proper coverage for CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE,
+# the kernel uses only C99 flexible arrays for dynamically sized trailing
+# arrays. Enforce this for everything that may examine structure sizes and
+# perform bounds checking.
+KBUILD_CFLAGS += $(call cc-option, -fstrict-flex-arrays=3)
+
# disable stringop warnings in gcc 8+
KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
--
2.34.1
On Wed, May 17, 2023 at 04:28:04PM -0700, Kees Cook wrote:
> The -fstrict-flex-arrays=3 option is now available with the release
> of GCC 13[1] and Clang 16[2]. This feature instructs the compiler to
> treat only C99 flexible arrays as dynamically sized for the purposes of
> object size calculations. In other words, the ancient practice of using
> 1-element arrays, or the GNU extension of using 0-sized arrays, as a
> dynamically sized array is disabled. This allows CONFIG_UBSAN_BOUNDS,
> CONFIG_FORTIFY_SOURCE, and other object-size aware features to behave
> unambiguously in the face of trailing arrays: only C99 flexible arrays
> are considered to be dynamically sized.
It's happening! :'-)
>
> Enabling this will help track down any outstanding cases of fake
> flexible arrays that need attention in kernel code.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays
> [2] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fstrict-flex-arrays
>
> Cc: Masahiro Yamada <[email protected]>
> Cc: "Gustavo A. R. Silva" <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Nicolas Schier <[email protected]>
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Thanks!
--
Gustavo
> ---
> Makefile | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index f836936fb4d8..07e5aec1daf5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1026,6 +1026,12 @@ KBUILD_CFLAGS += -Wno-pointer-sign
> # globally built with -Wcast-function-type.
> KBUILD_CFLAGS += $(call cc-option, -Wcast-function-type)
>
> +# To gain proper coverage for CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE,
> +# the kernel uses only C99 flexible arrays for dynamically sized trailing
> +# arrays. Enforce this for everything that may examine structure sizes and
> +# perform bounds checking.
> +KBUILD_CFLAGS += $(call cc-option, -fstrict-flex-arrays=3)
> +
> # disable stringop warnings in gcc 8+
> KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
>
> --
> 2.34.1
>
Kees Cook <[email protected]> writes:
> The -fstrict-flex-arrays=3 option is now available with the release
> of GCC 13[1] and Clang 16[2]. This feature instructs the compiler to
> treat only C99 flexible arrays as dynamically sized for the purposes of
> object size calculations. In other words, the ancient practice of using
> 1-element arrays, or the GNU extension of using 0-sized arrays, as a
> dynamically sized array is disabled. This allows CONFIG_UBSAN_BOUNDS,
> CONFIG_FORTIFY_SOURCE, and other object-size aware features to behave
> unambiguously in the face of trailing arrays: only C99 flexible arrays
> are considered to be dynamically sized.
>
> Enabling this will help track down any outstanding cases of fake
> flexible arrays that need attention in kernel code.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays
> [2] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fstrict-flex-arrays
>
Maybe link to https://people.kernel.org/kees/bounded-flexible-arrays-in-c as well
just in case some confused soul ends up bisecting to this but doesn't
get the problem?
Not really required though, just a thought I had.
On Thu, May 18, 2023 at 12:47:41AM +0100, Sam James wrote:
>
> Kees Cook <[email protected]> writes:
>
> > The -fstrict-flex-arrays=3 option is now available with the release
> > of GCC 13[1] and Clang 16[2]. This feature instructs the compiler to
> > treat only C99 flexible arrays as dynamically sized for the purposes of
> > object size calculations. In other words, the ancient practice of using
> > 1-element arrays, or the GNU extension of using 0-sized arrays, as a
> > dynamically sized array is disabled. This allows CONFIG_UBSAN_BOUNDS,
> > CONFIG_FORTIFY_SOURCE, and other object-size aware features to behave
> > unambiguously in the face of trailing arrays: only C99 flexible arrays
> > are considered to be dynamically sized.
> >
> > Enabling this will help track down any outstanding cases of fake
> > flexible arrays that need attention in kernel code.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays
> > [2] https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fstrict-flex-arrays
> >
>
> Maybe link to https://people.kernel.org/kees/bounded-flexible-arrays-in-c as well
> just in case some confused soul ends up bisecting to this but doesn't
> get the problem?
>
> Not really required though, just a thought I had.
Ah yeah, good idea! :)
--
Kees Cook
Hi Kees,
kernel test robot noticed the following build warnings:
[auto build test WARNING on masahiroy-kbuild/for-next]
[also build test WARNING on masahiroy-kbuild/fixes kees/for-next/pstore kees/for-next/kspp linus/master v6.4-rc2 next-20230519]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/kbuild-Enable-fstrict-flex-arrays-3/20230518-073007
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
patch link: https://lore.kernel.org/r/20230517232801.never.262-kees%40kernel.org
patch subject: [PATCH] kbuild: Enable -fstrict-flex-arrays=3
config: powerpc-allyesconfig
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b0fb98227c90adf2536c9ad644a74d5e92961111)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/83f5dbb6ef858d687130d8a2b122ebb792b2bd73
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kees-Cook/kbuild-Enable-fstrict-flex-arrays-3/20230518-073007
git checkout 83f5dbb6ef858d687130d8a2b122ebb792b2bd73
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/media/i2c/ drivers/scsi/ drivers/tty/serial/ drivers/usb/phy/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/scsi/BusLogic.c:51:
>> drivers/scsi/FlashPoint.c:1712:12: warning: stack frame size (1056) exceeds limit (1024) in 'FlashPoint_HandleInterrupt' [-Wframe-larger-than]
static int FlashPoint_HandleInterrupt(void *pcard)
^
1024/1056 (96.97%) spills, 32/1056 (3.03%) variables
1 warning generated.
vim +/FlashPoint_HandleInterrupt +1712 drivers/scsi/FlashPoint.c
^1da177e4c3f415 Linus Torvalds 2005-04-16 1702
^1da177e4c3f415 Linus Torvalds 2005-04-16 1703 /*---------------------------------------------------------------------
^1da177e4c3f415 Linus Torvalds 2005-04-16 1704 *
d8b6b8bd8a99ee9 Alexey Dobriyan 2006-03-08 1705 * Function: FlashPoint_HandleInterrupt
^1da177e4c3f415 Linus Torvalds 2005-04-16 1706 *
^1da177e4c3f415 Linus Torvalds 2005-04-16 1707 * Description: This is our entry point when an interrupt is generated
^1da177e4c3f415 Linus Torvalds 2005-04-16 1708 * by the card and the upper level driver passes it on to
^1da177e4c3f415 Linus Torvalds 2005-04-16 1709 * us.
^1da177e4c3f415 Linus Torvalds 2005-04-16 1710 *
^1da177e4c3f415 Linus Torvalds 2005-04-16 1711 *---------------------------------------------------------------------*/
391e2f25601e34a Khalid Aziz 2013-05-16 @1712 static int FlashPoint_HandleInterrupt(void *pcard)
^1da177e4c3f415 Linus Torvalds 2005-04-16 1713 {
69eb2ea47793366 Alexey Dobriyan 2006-03-08 1714 struct sccb *currSCCB;
554b117e8fab4f7 Colin Ian King 2022-07-30 1715 unsigned char thisCard, result, bm_status;
c823feeb33161c0 Alexey Dobriyan 2006-03-08 1716 unsigned short hp_int;
db038cf86fc63d3 Alexey Dobriyan 2006-03-08 1717 unsigned char i, target;
391e2f25601e34a Khalid Aziz 2013-05-16 1718 struct sccb_card *pCurrCard = pcard;
391e2f25601e34a Khalid Aziz 2013-05-16 1719 u32 ioport;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1720
391e2f25601e34a Khalid Aziz 2013-05-16 1721 thisCard = pCurrCard->cardIndex;
391e2f25601e34a Khalid Aziz 2013-05-16 1722 ioport = pCurrCard->ioPort;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1723
^1da177e4c3f415 Linus Torvalds 2005-04-16 1724 MDISABLE_INT(ioport);
^1da177e4c3f415 Linus Torvalds 2005-04-16 1725
554b117e8fab4f7 Colin Ian King 2022-07-30 1726 if (RD_HARPOON(ioport + hp_int_status) & EXT_STATUS_ON)
391e2f25601e34a Khalid Aziz 2013-05-16 1727 bm_status = RD_HARPOON(ioport + hp_ext_status) &
391e2f25601e34a Khalid Aziz 2013-05-16 1728 (unsigned char)BAD_EXT_STATUS;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1729 else
^1da177e4c3f415 Linus Torvalds 2005-04-16 1730 bm_status = 0;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1731
^1da177e4c3f415 Linus Torvalds 2005-04-16 1732 WR_HARPOON(ioport + hp_int_mask, (INT_CMD_COMPL | SCSI_INTERRUPT));
^1da177e4c3f415 Linus Torvalds 2005-04-16 1733
391e2f25601e34a Khalid Aziz 2013-05-16 1734 while ((hp_int = RDW_HARPOON((ioport + hp_intstat)) &
391e2f25601e34a Khalid Aziz 2013-05-16 1735 FPT_default_intena) | bm_status) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1736
391e2f25601e34a Khalid Aziz 2013-05-16 1737 currSCCB = pCurrCard->currentSCCB;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1738
^1da177e4c3f415 Linus Torvalds 2005-04-16 1739 if (hp_int & (FIFO | TIMEOUT | RESET | SCAM_SEL) || bm_status) {
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1740 result =
391e2f25601e34a Khalid Aziz 2013-05-16 1741 FPT_SccbMgr_bad_isr(ioport, thisCard, pCurrCard,
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1742 hp_int);
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1743 WRW_HARPOON((ioport + hp_intstat),
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1744 (FIFO | TIMEOUT | RESET | SCAM_SEL));
^1da177e4c3f415 Linus Torvalds 2005-04-16 1745 bm_status = 0;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1746
^1da177e4c3f415 Linus Torvalds 2005-04-16 1747 if (result) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1748
^1da177e4c3f415 Linus Torvalds 2005-04-16 1749 MENABLE_INT(ioport);
5c1b85e209af41c Alexey Dobriyan 2006-03-08 1750 return result;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1751 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 1752 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 1753
^1da177e4c3f415 Linus Torvalds 2005-04-16 1754 else if (hp_int & ICMD_COMP) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1755
^1da177e4c3f415 Linus Torvalds 2005-04-16 1756 if (!(hp_int & BUS_FREE)) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1757 /* Wait for the BusFree before starting a new command. We
^1da177e4c3f415 Linus Torvalds 2005-04-16 1758 must also check for being reselected since the BusFree
^1da177e4c3f415 Linus Torvalds 2005-04-16 1759 may not show up if another device reselects us in 1.5us or
^1da177e4c3f415 Linus Torvalds 2005-04-16 1760 less. SRR Wednesday, 3/8/1995.
^1da177e4c3f415 Linus Torvalds 2005-04-16 1761 */
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1762 while (!
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1763 (RDW_HARPOON((ioport + hp_intstat)) &
5c04a7b8981f285 Alexey Dobriyan 2006-03-08 1764 (BUS_FREE | RSEL))) ;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1765 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 1766
391e2f25601e34a Khalid Aziz 2013-05-16 1767 if (pCurrCard->globalFlags & F_HOST_XFER_ACT)
^1da177e4c3f415 Linus Torvalds 2005-04-16 1768
47b5d69c4aa753f James Bottomley 2005-04-24 1769 FPT_phaseChkFifo(ioport, thisCard);
^1da177e4c3f415 Linus Torvalds 2005-04-16 1770
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki