Message-ID: <CANpmjNOHSanxX7EyXhia4AuVd+6q5v1mXQMTM_k0Rj20P_ASAA@mail.gmail.com>
Date: Thu, 13 Mar 2025 11:29:29 -0400
From: Marco Elver <elver@...gle.com>
To: Kees Cook <kees@...nel.org>
Cc: Justin Stitt <justinstitt@...gle.com>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Andrey Konovalov <andreyknvl@...il.com>, 
	Andrey Ryabinin <ryabinin.a.a@...il.com>, Masahiro Yamada <masahiroy@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, Miguel Ojeda <ojeda@...nel.org>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, linux-hardening@...r.kernel.org, 
	kasan-dev@...glegroups.com, linux-kbuild@...r.kernel.org, 
	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 1/3] ubsan/overflow: Rework integer overflow sanitizer
 option to turn on everything

On Thu, 6 Mar 2025 at 23:19, Kees Cook <kees@...nel.org> wrote:
>
> Since we're going to approach integer overflow mitigation a type at a
> time, we need to enable all of the associated sanitizers, and then opt
> into types one at a time.
>
> Rename the existing "signed wrap" sanitizer to just the entire topic area:
> "integer wrap". Enable the implicit integer truncation sanitizers, with
> required callbacks and tests.
>
> Notably, this requires features (currently) only available in Clang,
> so we can depend on the cc-option tests to determine availability
> instead of doing version tests.
>
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> Cc: Justin Stitt <justinstitt@...gle.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Marco Elver <elver@...gle.com>
> Cc: Andrey Konovalov <andreyknvl@...il.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nicolas Schier <nicolas@...sle.eu>
> Cc: Miguel Ojeda <ojeda@...nel.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Hao Luo <haoluo@...gle.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Cc: linux-hardening@...r.kernel.org
> Cc: kasan-dev@...glegroups.com
> Cc: linux-kbuild@...r.kernel.org
> ---
>  include/linux/compiler_types.h  |  2 +-
>  kernel/configs/hardening.config |  2 +-
>  lib/Kconfig.ubsan               | 23 +++++++++++------------
>  lib/test_ubsan.c                | 18 ++++++++++++++----
>  lib/ubsan.c                     | 28 ++++++++++++++++++++++++++--
>  lib/ubsan.h                     |  8 ++++++++
>  scripts/Makefile.lib            |  4 ++--
>  scripts/Makefile.ubsan          |  8 ++++++--
>  8 files changed, 69 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index f59393464ea7..4ad3e900bc3d 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -360,7 +360,7 @@ struct ftrace_likely_data {
>  #endif
>
>  /* Do not trap wrapping arithmetic within an annotated function. */
> -#ifdef CONFIG_UBSAN_SIGNED_WRAP
> +#ifdef CONFIG_UBSAN_INTEGER_WRAP
>  # define __signed_wrap __attribute__((no_sanitize("signed-integer-overflow")))
>  #else
>  # define __signed_wrap
> diff --git a/kernel/configs/hardening.config b/kernel/configs/hardening.config
> index 3fabb8f55ef6..dd7c32fb5ac1 100644
> --- a/kernel/configs/hardening.config
> +++ b/kernel/configs/hardening.config
> @@ -46,7 +46,7 @@ CONFIG_UBSAN_BOUNDS=y
>  # CONFIG_UBSAN_SHIFT is not set
>  # CONFIG_UBSAN_DIV_ZERO is not set
>  # CONFIG_UBSAN_UNREACHABLE is not set
> -# CONFIG_UBSAN_SIGNED_WRAP is not set
> +# CONFIG_UBSAN_INTEGER_WRAP is not set
>  # CONFIG_UBSAN_BOOL is not set
>  # CONFIG_UBSAN_ENUM is not set
>  # CONFIG_UBSAN_ALIGNMENT is not set
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 1d4aa7a83b3a..63e5622010e0 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -116,21 +116,20 @@ config UBSAN_UNREACHABLE
>           This option enables -fsanitize=unreachable which checks for control
>           flow reaching an expected-to-be-unreachable position.
>
> -config UBSAN_SIGNED_WRAP
> -       bool "Perform checking for signed arithmetic wrap-around"
> +config UBSAN_INTEGER_WRAP
> +       bool "Perform checking for integer arithmetic wrap-around"
>         default UBSAN
>         depends on !COMPILE_TEST
> -       # The no_sanitize attribute was introduced in GCC with version 8.
> -       depends on !CC_IS_GCC || GCC_VERSION >= 80000
>         depends on $(cc-option,-fsanitize=signed-integer-overflow)
> -       help
> -         This option enables -fsanitize=signed-integer-overflow which checks
> -         for wrap-around of any arithmetic operations with signed integers.
> -         This currently performs nearly no instrumentation due to the
> -         kernel's use of -fno-strict-overflow which converts all would-be
> -         arithmetic undefined behavior into wrap-around arithmetic. Future
> -         sanitizer versions will allow for wrap-around checking (rather than
> -         exclusively undefined behavior).
> +       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)

Can these be in 1 cc-option? I know it might look slightly more ugly,
but having 3 different ones will shell out to the compiler 3 times,
which is a little less efficient. At some point it might noticeably
increase the build initialization latency.

Powered by blists - more mailing lists