linux-kernel - Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with clang

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Jan 2021 13:57:15 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Nathan Chancellor <natechancellor@...il.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Marco Elver <elver@...gle.com>,
        George Popescu <georgepope@...roid.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        linux-kernel@...r.kernel.org, clang-built-linux@...glegroups.com
Subject: Re: [PATCH] ubsan: disable unsigned-integer-overflow sanitizer with
 clang

On Wed, Dec 30, 2020 at 04:47:35PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.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@...db.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