* [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang @ 2020-12-30 15:47 Arnd Bergmann 2020-12-30 16:13 ` Marco Elver 2021-01-06 21:57 ` Kees Cook 0 siblings, 2 replies; 18+ messages in thread From: Arnd Bergmann @ 2020-12-30 15:47 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux From: Arnd Bergmann <arnd@arndb.de> Building ubsan kernels even for compile-testing introduced these warnings in my randconfig environment: crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] static void blake2b_compress(struct blake2b_state *S, crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) Further testing showed that this is caused by -fsanitize=unsigned-integer-overflow. The one in blake2b immediately overflows the 8KB stack area on 32-bit architectures, so better ensure this never happens by making this option gcc-only. Fixes: d0a3ac549f38 ("ubsan: enable for all*config builds") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- lib/Kconfig.ubsan | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index 8b635fd75fe4..e23873282ba7 100644 --- a/lib/Kconfig.ubsan +++ b/lib/Kconfig.ubsan @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW config UBSAN_UNSIGNED_OVERFLOW bool "Perform checking for unsigned arithmetic overflow" + # clang hugely expands stack usage with -fsanitize=object-size + depends on !CC_IS_CLANG depends on $(cc-option,-fsanitize=unsigned-integer-overflow) help This option enables -fsanitize=unsigned-integer-overflow which checks -- 2.29.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2020-12-30 15:47 [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang Arnd Bergmann @ 2020-12-30 16:13 ` Marco Elver 2021-01-04 22:33 ` Nathan Chancellor 2021-01-06 21:57 ` Kees Cook 1 sibling, 1 reply; 18+ messages in thread From: Marco Elver @ 2020-12-30 16:13 UTC (permalink / raw) To: Arnd Bergmann Cc: Kees Cook, Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > Building ubsan kernels even for compile-testing introduced these > warnings in my randconfig environment: > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > static void blake2b_compress(struct blake2b_state *S, > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > Further testing showed that this is caused by > -fsanitize=unsigned-integer-overflow. > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > architectures, so better ensure this never happens by making this > option gcc-only. > > Fixes: d0a3ac549f38 ("ubsan: enable for all*config builds") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > lib/Kconfig.ubsan | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > index 8b635fd75fe4..e23873282ba7 100644 > --- a/lib/Kconfig.ubsan > +++ b/lib/Kconfig.ubsan > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > config UBSAN_UNSIGNED_OVERFLOW > bool "Perform checking for unsigned arithmetic overflow" > + # clang hugely expands stack usage with -fsanitize=object-size This is the first time -fsanitize=object-size is mentioned. Typo? > + depends on !CC_IS_CLANG > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > help > This option enables -fsanitize=unsigned-integer-overflow which checks > -- > 2.29.2 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2020-12-30 16:13 ` Marco Elver @ 2021-01-04 22:33 ` Nathan Chancellor 2021-01-04 23:33 ` Andrew Morton 2021-01-05 9:25 ` Arnd Bergmann 0 siblings, 2 replies; 18+ messages in thread From: Nathan Chancellor @ 2021-01-04 22:33 UTC (permalink / raw) To: Marco Elver Cc: Arnd Bergmann, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > Building ubsan kernels even for compile-testing introduced these > > warnings in my randconfig environment: > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > static void blake2b_compress(struct blake2b_state *S, > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > Further testing showed that this is caused by > > -fsanitize=unsigned-integer-overflow. > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > architectures, so better ensure this never happens by making this > > option gcc-only. This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you sent a patch for [1], along with a couple of other issues I see such as: ld.lld: error: undefined symbol: __bad_mask >>> referenced by gpi.c >>> dma/qcom/gpi.o:(gpi_update_reg) in archive >>> drivers/built-in.a >>> referenced by gpi.c >>> dma/qcom/gpi.o:(gpi_update_reg) in archive >>> drivers/built-in.a [1]: https://lore.kernel.org/lkml/20201230154104.522605-1-arnd@kernel.org/ Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > > Fixes: d0a3ac549f38 ("ubsan: enable for all*config builds") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > lib/Kconfig.ubsan | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > index 8b635fd75fe4..e23873282ba7 100644 > > --- a/lib/Kconfig.ubsan > > +++ b/lib/Kconfig.ubsan > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > config UBSAN_UNSIGNED_OVERFLOW > > bool "Perform checking for unsigned arithmetic overflow" > > + # clang hugely expands stack usage with -fsanitize=object-size > > This is the first time -fsanitize=object-size is mentioned. Typo? Copy and paste issue from CONFIG_UBSAN_OBJECT_SIZE > > + depends on !CC_IS_CLANG > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > help > > This option enables -fsanitize=unsigned-integer-overflow which checks > > -- > > 2.29.2 > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-04 22:33 ` Nathan Chancellor @ 2021-01-04 23:33 ` Andrew Morton 2021-01-04 23:34 ` Nathan Chancellor 2021-01-05 9:25 ` Arnd Bergmann 1 sibling, 1 reply; 18+ messages in thread From: Andrew Morton @ 2021-01-04 23:33 UTC (permalink / raw) To: Nathan Chancellor Cc: Marco Elver, Arnd Bergmann, Kees Cook, Arnd Bergmann, Nick Desaulniers, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Mon, 4 Jan 2021 15:33:36 -0700 Nathan Chancellor <natechancellor@gmail.com> wrote: > > > +++ b/lib/Kconfig.ubsan > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > bool "Perform checking for unsigned arithmetic overflow" > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > > This is the first time -fsanitize=object-size is mentioned. Typo? > > Copy and paste issue from CONFIG_UBSAN_OBJECT_SIZE This? --- a/lib/Kconfig.ubsan~ubsan-disable-unsigned-integer-overflow-sanitizer-with-clang-fix +++ a/lib/Kconfig.ubsan @@ -122,7 +122,7 @@ config UBSAN_SIGNED_OVERFLOW config UBSAN_UNSIGNED_OVERFLOW bool "Perform checking for unsigned arithmetic overflow" - # clang hugely expands stack usage with -fsanitize=object-size + # clang hugely expands stack usage with -fsanitize=unsigned-integer-overflow depends on !CC_IS_CLANG depends on $(cc-option,-fsanitize=unsigned-integer-overflow) help _ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-04 23:33 ` Andrew Morton @ 2021-01-04 23:34 ` Nathan Chancellor 0 siblings, 0 replies; 18+ messages in thread From: Nathan Chancellor @ 2021-01-04 23:34 UTC (permalink / raw) To: Andrew Morton Cc: Marco Elver, Arnd Bergmann, Kees Cook, Arnd Bergmann, Nick Desaulniers, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Mon, Jan 04, 2021 at 03:33:33PM -0800, Andrew Morton wrote: > On Mon, 4 Jan 2021 15:33:36 -0700 Nathan Chancellor <natechancellor@gmail.com> wrote: > > > > > +++ b/lib/Kconfig.ubsan > > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > > bool "Perform checking for unsigned arithmetic overflow" > > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > > > > This is the first time -fsanitize=object-size is mentioned. Typo? > > > > Copy and paste issue from CONFIG_UBSAN_OBJECT_SIZE > > This? Correct. > --- a/lib/Kconfig.ubsan~ubsan-disable-unsigned-integer-overflow-sanitizer-with-clang-fix > +++ a/lib/Kconfig.ubsan > @@ -122,7 +122,7 @@ config UBSAN_SIGNED_OVERFLOW > > config UBSAN_UNSIGNED_OVERFLOW > bool "Perform checking for unsigned arithmetic overflow" > - # clang hugely expands stack usage with -fsanitize=object-size > + # clang hugely expands stack usage with -fsanitize=unsigned-integer-overflow > depends on !CC_IS_CLANG > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > help > _ > Cheers, Nathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-04 22:33 ` Nathan Chancellor 2021-01-04 23:33 ` Andrew Morton @ 2021-01-05 9:25 ` Arnd Bergmann 2021-01-06 9:12 ` Arnd Bergmann 1 sibling, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2021-01-05 9:25 UTC (permalink / raw) To: Nathan Chancellor Cc: Marco Elver, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Mon, Jan 4, 2021 at 11:33 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > Building ubsan kernels even for compile-testing introduced these > > > warnings in my randconfig environment: > > > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > > static void blake2b_compress(struct blake2b_state *S, > > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > > > Further testing showed that this is caused by > > > -fsanitize=unsigned-integer-overflow. > > > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > > architectures, so better ensure this never happens by making this > > > option gcc-only. > > This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you > sent a patch for [1], along with a couple of other issues I see such as: I'm fairly sure I still saw that BUILD_BUG() even after I had applied this patch, I would guess that one just depends on inlining decisions that are influenced by all kinds of compiler options including -fsanitize=unsigned-integer-overflow, so it becomes less likely. I'll revert my other patch in the randconfig tree to see if it comes back. Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-05 9:25 ` Arnd Bergmann @ 2021-01-06 9:12 ` Arnd Bergmann 2021-01-06 21:38 ` Nathan Chancellor 0 siblings, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2021-01-06 9:12 UTC (permalink / raw) To: Nathan Chancellor Cc: Marco Elver, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Tue, Jan 5, 2021 at 10:25 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jan 4, 2021 at 11:33 PM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > > > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > Building ubsan kernels even for compile-testing introduced these > > > > warnings in my randconfig environment: > > > > > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > > > static void blake2b_compress(struct blake2b_state *S, > > > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > > > > > Further testing showed that this is caused by > > > > -fsanitize=unsigned-integer-overflow. > > > > > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > > > architectures, so better ensure this never happens by making this > > > > option gcc-only. > > > > This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you > > sent a patch for [1], along with a couple of other issues I see such as: > > I'm fairly sure I still saw that BUILD_BUG() even after I had applied this > patch, I would guess that one just depends on inlining decisions that > are influenced by all kinds of compiler options including > -fsanitize=unsigned-integer-overflow, so it becomes less likely. > > I'll revert my other patch in the randconfig tree to see if it comes back. The qcom/gpi.c failure still happens with this patch applied: In file included from /git/arm-soc/drivers/dma/qcom/gpi.c:8: In function 'field_multiplier', inlined from 'gpi_update_reg' at /git/arm-soc/include/linux/bitfield.h:124:17: /git/arm-soc/include/linux/bitfield.h:119:3: error: call to '__bad_mask' declared with attribute error: bad bitfield mask 119 | __bad_mask(); | ^~~~~~~~~~~~ In function 'field_multiplier', inlined from 'gpi_update_reg' at /git/arm-soc/include/linux/bitfield.h:154:1: /git/arm-soc/include/linux/bitfield.h:119:3: error: call to '__bad_mask' declared with attribute error: bad bitfield mask 119 | __bad_mask(); | ^~~~~~~~~~~~ See https://pastebin.com/8UH6x4A2 for the .config Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 9:12 ` Arnd Bergmann @ 2021-01-06 21:38 ` Nathan Chancellor 2021-01-06 22:06 ` Arnd Bergmann 0 siblings, 1 reply; 18+ messages in thread From: Nathan Chancellor @ 2021-01-06 21:38 UTC (permalink / raw) To: Arnd Bergmann Cc: Marco Elver, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Wed, Jan 06, 2021 at 10:12:51AM +0100, Arnd Bergmann wrote: > On Tue, Jan 5, 2021 at 10:25 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > On Mon, Jan 4, 2021 at 11:33 PM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > > > On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > > > > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > > > Building ubsan kernels even for compile-testing introduced these > > > > > warnings in my randconfig environment: > > > > > > > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > > > > static void blake2b_compress(struct blake2b_state *S, > > > > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > > > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > > > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > > > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > > > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > > > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > > > > > > > Further testing showed that this is caused by > > > > > -fsanitize=unsigned-integer-overflow. > > > > > > > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > > > > architectures, so better ensure this never happens by making this > > > > > option gcc-only. > > > > > > This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you > > > sent a patch for [1], along with a couple of other issues I see such as: > > > > I'm fairly sure I still saw that BUILD_BUG() even after I had applied this > > patch, I would guess that one just depends on inlining decisions that > > are influenced by all kinds of compiler options including > > -fsanitize=unsigned-integer-overflow, so it becomes less likely. > > > > I'll revert my other patch in the randconfig tree to see if it comes back. > > The qcom/gpi.c failure still happens with this patch applied: > > In file included from /git/arm-soc/drivers/dma/qcom/gpi.c:8: > In function 'field_multiplier', > inlined from 'gpi_update_reg' at > /git/arm-soc/include/linux/bitfield.h:124:17: > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > '__bad_mask' declared with attribute error: bad bitfield mask > 119 | __bad_mask(); > | ^~~~~~~~~~~~ > In function 'field_multiplier', > inlined from 'gpi_update_reg' at > /git/arm-soc/include/linux/bitfield.h:154:1: > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > '__bad_mask' declared with attribute error: bad bitfield mask > 119 | __bad_mask(); > | ^~~~~~~~~~~~ > > See https://pastebin.com/8UH6x4A2 for the .config > > Arnd That config does not build for me, am I holding it wrong? $ curl -LSso .config https://pastebin.com/raw/8UH6x4A2 $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 olddefconfig vmlinux .config:364:warning: override: ARCH_DOVE changes choice state arch/arm/kernel/sys_oabi-compat.c:257:6: error: implicit declaration of function 'ep_op_has_event' [-Werror,-Wimplicit-function-declaration] if (ep_op_has_event(op) && ^ arch/arm/kernel/sys_oabi-compat.c:264:9: error: implicit declaration of function 'do_epoll_ctl' [-Werror,-Wimplicit-function-declaration] return do_epoll_ctl(epfd, op, fd, &kernel, false); ^ arch/arm/kernel/sys_oabi-compat.c:264:9: note: did you mean 'sys_epoll_ctl'? ./include/linux/syscalls.h:359:17: note: 'sys_epoll_ctl' declared here asmlinkage long sys_epoll_ctl(int epfd, int op, int fd, ^ 2 errors generated. ... Cheers, Nathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 21:38 ` Nathan Chancellor @ 2021-01-06 22:06 ` Arnd Bergmann 2021-01-07 3:34 ` Nathan Chancellor 0 siblings, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2021-01-06 22:06 UTC (permalink / raw) To: Nathan Chancellor Cc: Marco Elver, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Wed, Jan 6, 2021 at 10:38 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > On Wed, Jan 06, 2021 at 10:12:51AM +0100, Arnd Bergmann wrote: > > On Tue, Jan 5, 2021 at 10:25 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > On Mon, Jan 4, 2021 at 11:33 PM Nathan Chancellor > > > <natechancellor@gmail.com> wrote: > > > > On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > > > > > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > > > > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > > > > > Building ubsan kernels even for compile-testing introduced these > > > > > > warnings in my randconfig environment: > > > > > > > > > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > > > > > static void blake2b_compress(struct blake2b_state *S, > > > > > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > > > > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > > > > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > > > > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > > > > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > > > > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > > > > > > > > > Further testing showed that this is caused by > > > > > > -fsanitize=unsigned-integer-overflow. > > > > > > > > > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > > > > > architectures, so better ensure this never happens by making this > > > > > > option gcc-only. > > > > > > > > This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you > > > > sent a patch for [1], along with a couple of other issues I see such as: > > > > > > I'm fairly sure I still saw that BUILD_BUG() even after I had applied this > > > patch, I would guess that one just depends on inlining decisions that > > > are influenced by all kinds of compiler options including > > > -fsanitize=unsigned-integer-overflow, so it becomes less likely. > > > > > > I'll revert my other patch in the randconfig tree to see if it comes back. > > > > The qcom/gpi.c failure still happens with this patch applied: > > > > In file included from /git/arm-soc/drivers/dma/qcom/gpi.c:8: > > In function 'field_multiplier', > > inlined from 'gpi_update_reg' at > > /git/arm-soc/include/linux/bitfield.h:124:17: > > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > > '__bad_mask' declared with attribute error: bad bitfield mask > > 119 | __bad_mask(); > > | ^~~~~~~~~~~~ > > In function 'field_multiplier', > > inlined from 'gpi_update_reg' at > > /git/arm-soc/include/linux/bitfield.h:154:1: > > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > > '__bad_mask' declared with attribute error: bad bitfield mask > > 119 | __bad_mask(); > > | ^~~~~~~~~~~~ > > > > See https://pastebin.com/8UH6x4A2 for the .config > > > > Arnd > > That config does not build for me, am I holding it wrong? > > $ curl -LSso .config https://pastebin.com/raw/8UH6x4A2 Sorry about that, you ran into a bug that I have applied a local fix for. You could enable CONFIG_EPOLL, or apply this patch: https://lore.kernel.org/linux-arm-kernel/20200429132349.1294904-1-arnd@arndb.de/ Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 22:06 ` Arnd Bergmann @ 2021-01-07 3:34 ` Nathan Chancellor 0 siblings, 0 replies; 18+ messages in thread From: Nathan Chancellor @ 2021-01-07 3:34 UTC (permalink / raw) To: Arnd Bergmann Cc: Marco Elver, Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, George Popescu, Stephen Rothwell, LKML, clang-built-linux On Wed, Jan 06, 2021 at 11:06:39PM +0100, Arnd Bergmann wrote: > On Wed, Jan 6, 2021 at 10:38 PM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > On Wed, Jan 06, 2021 at 10:12:51AM +0100, Arnd Bergmann wrote: > > > On Tue, Jan 5, 2021 at 10:25 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > > > On Mon, Jan 4, 2021 at 11:33 PM Nathan Chancellor > > > > <natechancellor@gmail.com> wrote: > > > > > On Wed, Dec 30, 2020 at 05:13:03PM +0100, Marco Elver wrote: > > > > > > On Wed, 30 Dec 2020 at 16:47, Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > > > > > > > > > From: Arnd Bergmann <arnd@arndb.de> > > > > > > > > > > > > > > Building ubsan kernels even for compile-testing introduced these > > > > > > > warnings in my randconfig environment: > > > > > > > > > > > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > > > > > > static void blake2b_compress(struct blake2b_state *S, > > > > > > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > > > > > > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > > > > > > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > > > > > > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > > > > > > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > > > > > > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > > > > > > > > > > > > > Further testing showed that this is caused by > > > > > > > -fsanitize=unsigned-integer-overflow. > > > > > > > > > > > > > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > > > > > > > architectures, so better ensure this never happens by making this > > > > > > > option gcc-only. > > > > > > > > > > This patch also fixes the failed BUILD_BUG issue in mm/mremap.c that you > > > > > sent a patch for [1], along with a couple of other issues I see such as: > > > > > > > > I'm fairly sure I still saw that BUILD_BUG() even after I had applied this > > > > patch, I would guess that one just depends on inlining decisions that > > > > are influenced by all kinds of compiler options including > > > > -fsanitize=unsigned-integer-overflow, so it becomes less likely. > > > > > > > > I'll revert my other patch in the randconfig tree to see if it comes back. > > > > > > The qcom/gpi.c failure still happens with this patch applied: > > > > > > In file included from /git/arm-soc/drivers/dma/qcom/gpi.c:8: > > > In function 'field_multiplier', > > > inlined from 'gpi_update_reg' at > > > /git/arm-soc/include/linux/bitfield.h:124:17: > > > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > > > '__bad_mask' declared with attribute error: bad bitfield mask > > > 119 | __bad_mask(); > > > | ^~~~~~~~~~~~ > > > In function 'field_multiplier', > > > inlined from 'gpi_update_reg' at > > > /git/arm-soc/include/linux/bitfield.h:154:1: > > > /git/arm-soc/include/linux/bitfield.h:119:3: error: call to > > > '__bad_mask' declared with attribute error: bad bitfield mask > > > 119 | __bad_mask(); > > > | ^~~~~~~~~~~~ > > > > > > See https://pastebin.com/8UH6x4A2 for the .config > > > > > > Arnd > > > > That config does not build for me, am I holding it wrong? > > > > $ curl -LSso .config https://pastebin.com/raw/8UH6x4A2 > > Sorry about that, you ran into a bug that I have applied a > local fix for. You could enable CONFIG_EPOLL, or apply > this patch: > > https://lore.kernel.org/linux-arm-kernel/20200429132349.1294904-1-arnd@arndb.de/ > > Arnd I decided to just try to build the file directly. As it turns out, ARCH=arm allyesconfig does not show any error in get_extent or gpi_update_reg but ARCH=arm64 allyesconfig does. Looks like this is because of the lack of CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL for ARCH=arm. $ rg UBSAN out/{arm,arm64}/.config out/arm64/.config 13853:CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL=y 13854:CONFIG_UBSAN=y 13855:CONFIG_CC_HAS_UBSAN_BOUNDS=y 13856:CONFIG_CC_HAS_UBSAN_ARRAY_BOUNDS=y 13857:CONFIG_UBSAN_BOUNDS=y 13858:CONFIG_UBSAN_ARRAY_BOUNDS=y 13859:CONFIG_UBSAN_SHIFT=y 13860:CONFIG_UBSAN_DIV_ZERO=y 13861:CONFIG_UBSAN_UNREACHABLE=y 13862:CONFIG_UBSAN_SIGNED_OVERFLOW=y 13863:CONFIG_UBSAN_UNSIGNED_OVERFLOW=y 13864:CONFIG_UBSAN_OBJECT_SIZE=y 13865:CONFIG_UBSAN_BOOL=y 13866:CONFIG_UBSAN_ENUM=y 13867:CONFIG_UBSAN_SANITIZE_ALL=y 13868:CONFIG_TEST_UBSAN=m out/arm/.config 14133:CONFIG_UBSAN=y 14134:CONFIG_CC_HAS_UBSAN_BOUNDS=y 14135:CONFIG_CC_HAS_UBSAN_ARRAY_BOUNDS=y 14136:CONFIG_UBSAN_BOUNDS=y 14137:CONFIG_UBSAN_ARRAY_BOUNDS=y 14138:CONFIG_UBSAN_SHIFT=y 14139:CONFIG_UBSAN_DIV_ZERO=y 14140:CONFIG_UBSAN_UNREACHABLE=y 14141:CONFIG_UBSAN_SIGNED_OVERFLOW=y 14142:CONFIG_UBSAN_UNSIGNED_OVERFLOW=y 14143:CONFIG_UBSAN_OBJECT_SIZE=y 14144:CONFIG_UBSAN_BOOL=y 14145:CONFIG_UBSAN_ENUM=y 14146:CONFIG_TEST_UBSAN=m $ llvm-nm -S out/arm64/drivers/dma/qcom/gpi.o |& rg __bad_mask U __bad_mask $ llvm-nm -S out/arm/drivers/dma/qcom/gpi.o |& rg __bad_mask Applying this diff causes __bad_mask to show up: diff --git a/drivers/dma/qcom/Makefile b/drivers/dma/qcom/Makefile index 50f1e7014693..c261adb47960 100644 --- a/drivers/dma/qcom/Makefile +++ b/drivers/dma/qcom/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_QCOM_ADM) += qcom_adm.o obj-$(CONFIG_QCOM_BAM_DMA) += bam_dma.o obj-$(CONFIG_QCOM_GPI_DMA) += gpi.o +UBSAN_SANITIZE_gpi.o := y obj-$(CONFIG_QCOM_HIDMA_MGMT) += hdma_mgmt.o hdma_mgmt-objs := hidma_mgmt.o hidma_mgmt_sys.o obj-$(CONFIG_QCOM_HIDMA) += hdma.o $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 \ O=out/arm distclean allyesconfig drivers/dma/qcom/gpi.o $ llvm-nm -S out/arm/drivers/dma/qcom/gpi.o |& rg __bad_mask U __bad_mask So I guess I am curious how you saw this pop up. Not to mention it seems like that error is with GCC rather than clang? For what it's worth, when I run that .config through olddefconfig, CONFIG_ARCH_QCOM and CONFIG_QCOM_GPI_DMA get disabled... Am I doing something wrong? $ curl -LSso /tmp/out/arm/.config https://pastebin.com/raw/8UH6x4A2 $ rg "CONFIG_ARCH_QCOM|CONFIG_QCOM_GPI_DMA" /tmp/out/arm/.config 377:CONFIG_ARCH_QCOM=y 4437:CONFIG_QCOM_GPI_DMA=y $ make.sh ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- LLVM=1 O=/tmp/out/arm olddefconfig .config:364:warning: override: ARCH_DOVE changes choice state $ rg "CONFIG_ARCH_QCOM|CONFIG_QCOM_GPI_DMA" /tmp/out/arm/.config Cheers, Nathan ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2020-12-30 15:47 [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang Arnd Bergmann 2020-12-30 16:13 ` Marco Elver @ 2021-01-06 21:57 ` Kees Cook 2021-01-06 22:12 ` Arnd Bergmann 1 sibling, 1 reply; 18+ messages in thread From: Kees Cook @ 2021-01-06 21:57 UTC (permalink / raw) To: Arnd Bergmann Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Building ubsan kernels even for compile-testing introduced these > warnings in my randconfig environment: > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > static void blake2b_compress(struct blake2b_state *S, > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > static void sha512_generic_block_fn(struct sha512_state *sst, u8 const *src, > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > static noinline void fe_mul_impl(u32 out[10], const u32 in1[10], const u32 in2[10]) > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > static noinline void fe_sqr_impl(u32 out[10], const u32 in1[10]) > > Further testing showed that this is caused by > -fsanitize=unsigned-integer-overflow. > > The one in blake2b immediately overflows the 8KB stack area on 32-bit > architectures, so better ensure this never happens by making this > option gcc-only. > > Fixes: d0a3ac549f38 ("ubsan: enable for all*config builds") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > lib/Kconfig.ubsan | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > index 8b635fd75fe4..e23873282ba7 100644 > --- a/lib/Kconfig.ubsan > +++ b/lib/Kconfig.ubsan > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > config UBSAN_UNSIGNED_OVERFLOW > bool "Perform checking for unsigned arithmetic overflow" > + # clang hugely expands stack usage with -fsanitize=object-size > + depends on !CC_IS_CLANG > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) Because of Clang implementation issues (see commit c637693b20da), this is already "default n" (and not supported under GCC at all). IIUC, setting this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? Is there some better way to mark this as "known to have issues, please don't include in randconfig?" I'd like to keep it around so people can continue to work out the problems with it, but not have unexpecting folks trip over it. ;) -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 21:57 ` Kees Cook @ 2021-01-06 22:12 ` Arnd Bergmann 2021-01-06 23:17 ` Kees Cook 2021-01-07 16:09 ` Arnd Bergmann 0 siblings, 2 replies; 18+ messages in thread From: Arnd Bergmann @ 2021-01-06 22:12 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > index 8b635fd75fe4..e23873282ba7 100644 > > --- a/lib/Kconfig.ubsan > > +++ b/lib/Kconfig.ubsan > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > config UBSAN_UNSIGNED_OVERFLOW > > bool "Perform checking for unsigned arithmetic overflow" > > + # clang hugely expands stack usage with -fsanitize=object-size > > + depends on !CC_IS_CLANG > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > Because of Clang implementation issues (see commit c637693b20da), this is > already "default n" (and not supported under GCC at all). IIUC, setting > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST dependency would hide it for me, which may be good enough for me. > Is there some better way to mark this as "known to have issues, please > don't include in randconfig?" > > I'd like to keep it around so people can continue to work out the > problems with it, but not have unexpecting folks trip over it. ;) I've reverted that patch locally now and default-enabled for randconfigs. Now that I have an otherwise clean build, this should provide me with a full set of files that produce the warning. If the number is small enough, I could try opening individual github issues. Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 22:12 ` Arnd Bergmann @ 2021-01-06 23:17 ` Kees Cook 2021-01-06 23:40 ` Arnd Bergmann 2021-01-07 16:09 ` Arnd Bergmann 1 sibling, 1 reply; 18+ messages in thread From: Kees Cook @ 2021-01-06 23:17 UTC (permalink / raw) To: Arnd Bergmann Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Wed, Jan 06, 2021 at 11:12:18PM +0100, Arnd Bergmann wrote: > On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > > index 8b635fd75fe4..e23873282ba7 100644 > > > --- a/lib/Kconfig.ubsan > > > +++ b/lib/Kconfig.ubsan > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > bool "Perform checking for unsigned arithmetic overflow" > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > + depends on !CC_IS_CLANG > > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > > > Because of Clang implementation issues (see commit c637693b20da), this is > > already "default n" (and not supported under GCC at all). IIUC, setting > > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? > > Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST > dependency would hide it for me, which may be good enough for me. I thought COMPILE_TEST does not get set for randconfig? > > Is there some better way to mark this as "known to have issues, please > > don't include in randconfig?" > > > > I'd like to keep it around so people can continue to work out the > > problems with it, but not have unexpecting folks trip over it. ;) > > I've reverted that patch locally now and default-enabled for randconfigs. > Now that I have an otherwise clean build, this should provide me > with a full set of files that produce the warning. If the number is > small enough, I could try opening individual github issues. Okay, let me know if something needs changing. :) -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 23:17 ` Kees Cook @ 2021-01-06 23:40 ` Arnd Bergmann 0 siblings, 0 replies; 18+ messages in thread From: Arnd Bergmann @ 2021-01-06 23:40 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Thu, Jan 7, 2021 at 12:17 AM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Jan 06, 2021 at 11:12:18PM +0100, Arnd Bergmann wrote: > > On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > > > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > > > index 8b635fd75fe4..e23873282ba7 100644 > > > > --- a/lib/Kconfig.ubsan > > > > +++ b/lib/Kconfig.ubsan > > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > > bool "Perform checking for unsigned arithmetic overflow" > > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > > + depends on !CC_IS_CLANG > > > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > > > > > Because of Clang implementation issues (see commit c637693b20da), this is > > > already "default n" (and not supported under GCC at all). IIUC, setting > > > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? > > > > Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST > > dependency would hide it for me, which may be good enough for me. > > I thought COMPILE_TEST does not get set for randconfig? It does on my kernel, though I never submitted that patch ;-) Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-06 22:12 ` Arnd Bergmann 2021-01-06 23:17 ` Kees Cook @ 2021-01-07 16:09 ` Arnd Bergmann 2021-01-07 17:22 ` Kees Cook 1 sibling, 1 reply; 18+ messages in thread From: Arnd Bergmann @ 2021-01-07 16:09 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Wed, Jan 6, 2021 at 11:12 PM Arnd Bergmann <arnd@kernel.org> wrote: > > On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > > index 8b635fd75fe4..e23873282ba7 100644 > > > --- a/lib/Kconfig.ubsan > > > +++ b/lib/Kconfig.ubsan > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > bool "Perform checking for unsigned arithmetic overflow" > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > + depends on !CC_IS_CLANG > > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > > > Because of Clang implementation issues (see commit c637693b20da), this is > > already "default n" (and not supported under GCC at all). IIUC, setting > > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? > > Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST > dependency would hide it for me, which may be good enough for me. > > > Is there some better way to mark this as "known to have issues, please > > don't include in randconfig?" > > > > I'd like to keep it around so people can continue to work out the > > problems with it, but not have unexpecting folks trip over it. ;) > > I've reverted that patch locally now and default-enabled for randconfigs. > Now that I have an otherwise clean build, this should provide me > with a full set of files that produce the warning. If the number is > small enough, I could try opening individual github issues. A day's worth of randconfig builds with clang-11 or clang-12 shows these instances that exceeded the warning limit: crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] fs/btrfs/scrub.c:3028:31: error: stack frame size of 1132 bytes in function 'scrub_stripe' [-Werror,-Wframe-larger-than=] drivers/net/ethernet/intel/e1000/e1000_main.c:3590:6: warning: stack frame size of 1100 bytes in function 'e1000_update_stats' [-Wframe-larger-than=] drivers/net/ethernet/broadcom/tg3.c:11829:13: warning: stack frame size of 1188 bytes in function 'tg3_get_estats' [-Wframe-larger-than=] drivers/net/ethernet/intel/igb/igb_main.c:6551:6: warning: stack frame size of 1348 bytes in function 'igb_update_stats' [-Wframe-larger-than=] drivers/net/ethernet/intel/igc/igc_main.c:3608:6: warning: stack frame size of 1124 bytes in function 'igc_update_stats' [-Wframe-larger-than=] drivers/net/ethernet/qlogic/qed/qed_l2.c:1759:1: warning: stack frame size of 1300 bytes in function '__qed_get_vport_port_stats' [-Wframe-larger-than=] drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7022:6: warning: stack frame size of 1564 bytes in function 'ixgbe_update_stats' [-Wframe-larger-than=] drivers/net/ethernet/intel/ixgb/ixgb_main.c:1590:1: warning: stack frame size of 1140 bytes in function 'ixgb_update_stats' [-Wframe-larger-than=] drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:145:6: warning: stack frame size of 1068 bytes in function 'mlx5i_get_stats' [-Wframe-larger-than=] drivers/staging/media/atomisp/pci/atomisp_cmd.c:5600:5: warning: stack frame size of 1052 bytes in function 'atomisp_set_fmt' [-Wframe-larger-than=] All of these *only* happen on 32-bit x86, and can be reproduced in a i386_defconfig, with the corresponding drivers (btrfs, sha512, blake2b, curve25519, and the ethernet ones) and UBSAN_UNSIGNED_OVERFLOW manually enabled. Given that few people still care about i386, maybe we could just make the option depend on !CONFIG_X86_32 That config also runs into two more BUILD_BUG_ON() that I had not seen in randconfig tests: (i386 defconfig plus ubsan) ld.lld: error: undefined symbol: __compiletime_assert_207 >>> referenced by cpu_entry_area.c >>> mm/cpu_entry_area.o:(setup_cpu_entry_area_ptes) in archive arch/x86/built-in.a (x86-64 defconfig plus ubsan) ld.lld: error: undefined symbol: __compiletime_assert_362 >>> referenced by efi_64.c >>> platform/efi/efi_64.o:(efi_sync_low_kernel_mappings) in archive arch/x86/built-in.a Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-07 16:09 ` Arnd Bergmann @ 2021-01-07 17:22 ` Kees Cook 2021-01-07 18:15 ` Nathan Chancellor 0 siblings, 1 reply; 18+ messages in thread From: Kees Cook @ 2021-01-07 17:22 UTC (permalink / raw) To: Arnd Bergmann Cc: Arnd Bergmann, Nathan Chancellor, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Thu, Jan 07, 2021 at 05:09:59PM +0100, Arnd Bergmann wrote: > On Wed, Jan 6, 2021 at 11:12 PM Arnd Bergmann <arnd@kernel.org> wrote: > > > > On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > > > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > > > index 8b635fd75fe4..e23873282ba7 100644 > > > > --- a/lib/Kconfig.ubsan > > > > +++ b/lib/Kconfig.ubsan > > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > > bool "Perform checking for unsigned arithmetic overflow" > > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > > + depends on !CC_IS_CLANG > > > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > > > > > Because of Clang implementation issues (see commit c637693b20da), this is > > > already "default n" (and not supported under GCC at all). IIUC, setting > > > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? > > > > Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST > > dependency would hide it for me, which may be good enough for me. > > > > > Is there some better way to mark this as "known to have issues, please > > > don't include in randconfig?" > > > > > > I'd like to keep it around so people can continue to work out the > > > problems with it, but not have unexpecting folks trip over it. ;) > > > > I've reverted that patch locally now and default-enabled for randconfigs. > > Now that I have an otherwise clean build, this should provide me > > with a full set of files that produce the warning. If the number is > > small enough, I could try opening individual github issues. > > A day's worth of randconfig builds with clang-11 or clang-12 shows these > instances that exceeded the warning limit: > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes > in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes > in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 > bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 > bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > fs/btrfs/scrub.c:3028:31: error: stack frame size of 1132 bytes in > function 'scrub_stripe' [-Werror,-Wframe-larger-than=] > drivers/net/ethernet/intel/e1000/e1000_main.c:3590:6: warning: stack > frame size of 1100 bytes in function 'e1000_update_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/broadcom/tg3.c:11829:13: warning: stack frame > size of 1188 bytes in function 'tg3_get_estats' [-Wframe-larger-than=] > drivers/net/ethernet/intel/igb/igb_main.c:6551:6: warning: stack frame > size of 1348 bytes in function 'igb_update_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/intel/igc/igc_main.c:3608:6: warning: stack frame > size of 1124 bytes in function 'igc_update_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/qlogic/qed/qed_l2.c:1759:1: warning: stack frame > size of 1300 bytes in function '__qed_get_vport_port_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7022:6: warning: stack > frame size of 1564 bytes in function 'ixgbe_update_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/intel/ixgb/ixgb_main.c:1590:1: warning: stack > frame size of 1140 bytes in function 'ixgb_update_stats' > [-Wframe-larger-than=] > drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:145:6: warning: > stack frame size of 1068 bytes in function 'mlx5i_get_stats' > [-Wframe-larger-than=] > drivers/staging/media/atomisp/pci/atomisp_cmd.c:5600:5: warning: stack > frame size of 1052 bytes in function 'atomisp_set_fmt' > [-Wframe-larger-than=] > > All of these *only* happen on 32-bit x86, and can be reproduced in a > i386_defconfig, with the corresponding drivers (btrfs, sha512, blake2b, > curve25519, and the ethernet ones) and UBSAN_UNSIGNED_OVERFLOW > manually enabled. Given that few people still care about i386, maybe > we could just make the option depend on !CONFIG_X86_32 I'm fine with that -- or maybe any 32-bit architecture, if the problem is poor stack space optimization on 32-bit archs? > > That config also runs into two more BUILD_BUG_ON() that I had not > seen in randconfig tests: > > (i386 defconfig plus ubsan) > ld.lld: error: undefined symbol: __compiletime_assert_207 > >>> referenced by cpu_entry_area.c > >>> mm/cpu_entry_area.o:(setup_cpu_entry_area_ptes) in archive arch/x86/built-in.a That one I don't think I've seen before. > > (x86-64 defconfig plus ubsan) > ld.lld: error: undefined symbol: __compiletime_assert_362 > >>> referenced by efi_64.c > >>> platform/efi/efi_64.o:(efi_sync_low_kernel_mappings) in archive arch/x86/built-in.a I think this is: https://github.com/ClangBuiltLinux/linux/issues/256 and that bug needs re-opening? Or maybe there's a new bug I can't find? -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-07 17:22 ` Kees Cook @ 2021-01-07 18:15 ` Nathan Chancellor 2021-01-07 20:41 ` Arnd Bergmann 0 siblings, 1 reply; 18+ messages in thread From: Nathan Chancellor @ 2021-01-07 18:15 UTC (permalink / raw) To: Kees Cook Cc: Arnd Bergmann, Arnd Bergmann, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Thu, Jan 07, 2021 at 09:22:00AM -0800, Kees Cook wrote: > On Thu, Jan 07, 2021 at 05:09:59PM +0100, Arnd Bergmann wrote: > > On Wed, Jan 6, 2021 at 11:12 PM Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > On Wed, Jan 6, 2021 at 10:57 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote: > > > > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > > > > > index 8b635fd75fe4..e23873282ba7 100644 > > > > > --- a/lib/Kconfig.ubsan > > > > > +++ b/lib/Kconfig.ubsan > > > > > @@ -122,6 +122,8 @@ config UBSAN_SIGNED_OVERFLOW > > > > > > > > > > config UBSAN_UNSIGNED_OVERFLOW > > > > > bool "Perform checking for unsigned arithmetic overflow" > > > > > + # clang hugely expands stack usage with -fsanitize=object-size > > > > > + depends on !CC_IS_CLANG > > > > > depends on $(cc-option,-fsanitize=unsigned-integer-overflow) > > > > > > > > Because of Clang implementation issues (see commit c637693b20da), this is > > > > already "default n" (and not supported under GCC at all). IIUC, setting > > > > this to "depends on !COMPILE_TEST" won't work for randconfigs, yes? > > > > > > Ah, I had not realized this is clang specific. Adding the !COMPILE_TEST > > > dependency would hide it for me, which may be good enough for me. > > > > > > > Is there some better way to mark this as "known to have issues, please > > > > don't include in randconfig?" > > > > > > > > I'd like to keep it around so people can continue to work out the > > > > problems with it, but not have unexpecting folks trip over it. ;) > > > > > > I've reverted that patch locally now and default-enabled for randconfigs. > > > Now that I have an otherwise clean build, this should provide me > > > with a full set of files that produce the warning. If the number is > > > small enough, I could try opening individual github issues. > > > > A day's worth of randconfig builds with clang-11 or clang-12 shows these > > instances that exceeded the warning limit: > > > > crypto/blake2b_generic.c:98:13: error: stack frame size of 9636 bytes > > in function 'blake2b_compress' [-Werror,-Wframe-larger-than=] > > crypto/sha512_generic.c:151:13: error: stack frame size of 1292 bytes > > in function 'sha512_generic_block_fn' [-Werror,-Wframe-larger-than=] > > lib/crypto/curve25519-fiat32.c:312:22: error: stack frame size of 2180 > > bytes in function 'fe_mul_impl' [-Werror,-Wframe-larger-than=] > > lib/crypto/curve25519-fiat32.c:444:22: error: stack frame size of 1588 > > bytes in function 'fe_sqr_impl' [-Werror,-Wframe-larger-than=] > > fs/btrfs/scrub.c:3028:31: error: stack frame size of 1132 bytes in > > function 'scrub_stripe' [-Werror,-Wframe-larger-than=] > > drivers/net/ethernet/intel/e1000/e1000_main.c:3590:6: warning: stack > > frame size of 1100 bytes in function 'e1000_update_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/broadcom/tg3.c:11829:13: warning: stack frame > > size of 1188 bytes in function 'tg3_get_estats' [-Wframe-larger-than=] > > drivers/net/ethernet/intel/igb/igb_main.c:6551:6: warning: stack frame > > size of 1348 bytes in function 'igb_update_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/intel/igc/igc_main.c:3608:6: warning: stack frame > > size of 1124 bytes in function 'igc_update_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/qlogic/qed/qed_l2.c:1759:1: warning: stack frame > > size of 1300 bytes in function '__qed_get_vport_port_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7022:6: warning: stack > > frame size of 1564 bytes in function 'ixgbe_update_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/intel/ixgb/ixgb_main.c:1590:1: warning: stack > > frame size of 1140 bytes in function 'ixgb_update_stats' > > [-Wframe-larger-than=] > > drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:145:6: warning: > > stack frame size of 1068 bytes in function 'mlx5i_get_stats' > > [-Wframe-larger-than=] > > drivers/staging/media/atomisp/pci/atomisp_cmd.c:5600:5: warning: stack > > frame size of 1052 bytes in function 'atomisp_set_fmt' > > [-Wframe-larger-than=] > > > > All of these *only* happen on 32-bit x86, and can be reproduced in a > > i386_defconfig, with the corresponding drivers (btrfs, sha512, blake2b, > > curve25519, and the ethernet ones) and UBSAN_UNSIGNED_OVERFLOW > > manually enabled. Given that few people still care about i386, maybe > > we could just make the option depend on !CONFIG_X86_32 > > I'm fine with that -- or maybe any 32-bit architecture, if the problem > is poor stack space optimization on 32-bit archs? > > > > > That config also runs into two more BUILD_BUG_ON() that I had not > > seen in randconfig tests: > > > > (i386 defconfig plus ubsan) > > ld.lld: error: undefined symbol: __compiletime_assert_207 > > >>> referenced by cpu_entry_area.c > > >>> mm/cpu_entry_area.o:(setup_cpu_entry_area_ptes) in archive arch/x86/built-in.a > > That one I don't think I've seen before. > > > > > (x86-64 defconfig plus ubsan) > > ld.lld: error: undefined symbol: __compiletime_assert_362 > > >>> referenced by efi_64.c > > >>> platform/efi/efi_64.o:(efi_sync_low_kernel_mappings) in archive arch/x86/built-in.a > > I think this is: > https://github.com/ClangBuiltLinux/linux/issues/256 > and that bug needs re-opening? Or maybe there's a new bug I can't find? The problem is that applying the fix for that issue does not work, nor does converting p4d_index to a macro like mips and s390. I am not sure what exactly is going on there, it appears that clang cannot reason about ptrs_per_p4d because it is an extern variable with no assigned value in its translation unit? Cheers, Nathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang 2021-01-07 18:15 ` Nathan Chancellor @ 2021-01-07 20:41 ` Arnd Bergmann 0 siblings, 0 replies; 18+ messages in thread From: Arnd Bergmann @ 2021-01-07 20:41 UTC (permalink / raw) To: Nathan Chancellor Cc: Kees Cook, Arnd Bergmann, Nick Desaulniers, Andrew Morton, Marco Elver, George Popescu, Stephen Rothwell, linux-kernel, clang-built-linux On Thu, Jan 7, 2021 at 7:15 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > On Thu, Jan 07, 2021 at 09:22:00AM -0800, Kees Cook wrote: > > I think this is: > > https://github.com/ClangBuiltLinux/linux/issues/256 > > and that bug needs re-opening? Or maybe there's a new bug I can't find? > > The problem is that applying the fix for that issue does not work, nor > does converting p4d_index to a macro like mips and s390. I am not sure > what exactly is going on there, it appears that clang cannot reason > about ptrs_per_p4d because it is an extern variable with no assigned > value in its translation unit? Right, I tried the __always_inline trick already and concluded the same. Arnd ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-01-07 20:42 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-30 15:47 [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang Arnd Bergmann 2020-12-30 16:13 ` Marco Elver 2021-01-04 22:33 ` Nathan Chancellor 2021-01-04 23:33 ` Andrew Morton 2021-01-04 23:34 ` Nathan Chancellor 2021-01-05 9:25 ` Arnd Bergmann 2021-01-06 9:12 ` Arnd Bergmann 2021-01-06 21:38 ` Nathan Chancellor 2021-01-06 22:06 ` Arnd Bergmann 2021-01-07 3:34 ` Nathan Chancellor 2021-01-06 21:57 ` Kees Cook 2021-01-06 22:12 ` Arnd Bergmann 2021-01-06 23:17 ` Kees Cook 2021-01-06 23:40 ` Arnd Bergmann 2021-01-07 16:09 ` Arnd Bergmann 2021-01-07 17:22 ` Kees Cook 2021-01-07 18:15 ` Nathan Chancellor 2021-01-07 20:41 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).