linux-kernel - Re: [PATCH 3/3] ubsan/overflow: Enable ignorelist parsing and add type filter

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] [day] [month] [year] [list]

Message-ID: <52samxs253u3t2cmm5xwbmrwzyof36w7xczpuvbkarqwonwl32@2jbmkagpk7za>
Date: Fri, 7 Mar 2025 15:39:57 -0800
From: Justin Stitt <justinstitt@...gle.com>
To: Kees Cook <kees@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	Marco Elver <elver@...gle.com>, Andrey Konovalov <andreyknvl@...il.com>, 
	Andrey Ryabinin <ryabinin.a.a@...il.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, kasan-dev@...glegroups.com, linux-hardening@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, Bill Wendling <morbo@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Tony Ambardar <tony.ambardar@...il.com>, 
	Alexander Potapenko <glider@...gle.com>, Jan Hendrik Farr <kernel@...rr.cc>, 
	Alexander Lobakin <aleksander.lobakin@...el.com>, linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH 3/3] ubsan/overflow: Enable ignorelist parsing and add
 type filter

On Thu, Mar 06, 2025 at 08:19:11PM -0800, Kees Cook wrote:
> Limit integer wrap-around mitigation to only the "size_t" type (for
> now). Notably this covers all special functions/builtins that return
> "size_t", like sizeof(). This remains an experimental feature and is
> likely to be replaced with type annotations.
> 
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> Cc: Justin Stitt <justinstitt@...gle.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> Cc: Marco Elver <elver@...gle.com>
> Cc: Andrey Konovalov <andreyknvl@...il.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nicolas Schier <nicolas@...sle.eu>
> Cc: kasan-dev@...glegroups.com
> Cc: linux-hardening@...r.kernel.org
> Cc: linux-kbuild@...r.kernel.org
> ---
>  lib/Kconfig.ubsan               | 1 +
>  scripts/Makefile.ubsan          | 3 ++-
>  scripts/integer-wrap-ignore.scl | 3 +++
>  3 files changed, 6 insertions(+), 1 deletion(-)
>  create mode 100644 scripts/integer-wrap-ignore.scl
> 
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 888c2e72c586..4216b3a4ff21 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -125,6 +125,7 @@ config UBSAN_INTEGER_WRAP
>  	depends on $(cc-option,-fsanitize=unsigned-integer-overflow)
>  	depends on $(cc-option,-fsanitize=implicit-signed-integer-truncation)
>  	depends on $(cc-option,-fsanitize=implicit-unsigned-integer-truncation)
> +	depends on $(cc-option,-fsanitize-ignorelist=/dev/null)
>  	help
>  	  This option enables all of the sanitizers involved in integer overflow
>  	  (wrap-around) mitigation: signed-integer-overflow, unsigned-integer-overflow,
> diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
> index 233379c193a7..9e35198edbf0 100644
> --- a/scripts/Makefile.ubsan
> +++ b/scripts/Makefile.ubsan
> @@ -19,5 +19,6 @@ ubsan-integer-wrap-cflags-$(CONFIG_UBSAN_INTEGER_WRAP)     +=	\
>  	-fsanitize=signed-integer-overflow			\
>  	-fsanitize=unsigned-integer-overflow			\
>  	-fsanitize=implicit-signed-integer-truncation		\
> -	-fsanitize=implicit-unsigned-integer-truncation
> +	-fsanitize=implicit-unsigned-integer-truncation		\
> +	-fsanitize-ignorelist=$(srctree)/scripts/integer-wrap-ignore.scl
>  export CFLAGS_UBSAN_INTEGER_WRAP := $(ubsan-integer-wrap-cflags-y)
> diff --git a/scripts/integer-wrap-ignore.scl b/scripts/integer-wrap-ignore.scl
> new file mode 100644
> index 000000000000..431c3053a4a2
> --- /dev/null
> +++ b/scripts/integer-wrap-ignore.scl
> @@ -0,0 +1,3 @@
> +[{unsigned-integer-overflow,signed-integer-overflow,implicit-signed-integer-truncation,implicit-unsigned-integer-truncation}]
> +type:*
> +type:size_t=sanitize

Forgot to mention this in my intial reply but we have to be careful
with what types are added here. Kees, I know we're on the same page from
offline chats but for others: using sanitizer case lists to discriminate
against types for the purposes of sanitizer instrumentation may not work
properly through various arithmetic conversions. Mainly, implicit
promotions which tend to break this particular approach.

Now, for size_t we got kind of "lucky" because there are no implicit
promotions with size_t, it doesn't get promoted. This is not the case
for other types. This further necessitates the need for canonical
wrapping types backed by in-source annotations/qualification -- coming
soon in Clang.

> -- 
> 2.34.1
> 

Justin