linux-kernel - Re: [PATCH 1/3] ubsan/overflow: Rework integer overflow sanitizer option to turn on everything

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: <2AACDA6E-F7EF-4962-937A-C9511E4E2930@kernel.org>
Date: Thu, 13 Mar 2025 17:04:24 -0700
From: Kees Cook <kees@...nel.org>
To: Marco Elver <elver@...gle.com>
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 March 13, 2025 8:29:29 AM PDT, Marco Elver <elver@...gle.com> wrote:
>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.

Yeah, good point. I could probably just test the most recently added option, as it implies all the rest, too. I'll send an update!

-Kees


-- 
Kees Cook