linux-kernel - Re: [PATCH v3] ubsan: Reintroduce signed overflow sanitizer

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]

Message-ID: <67a842ad-b900-4c63-afcb-63455934f727@gmail.com>
Date: Mon, 5 Feb 2024 13:54:24 +0100
From: Andrey Ryabinin <ryabinin.a.a@...il.com>
To: Kees Cook <keescook@...omium.org>, Justin Stitt <justinstitt@...gle.com>
Cc: Marco Elver <elver@...gle.com>, Miguel Ojeda <ojeda@...nel.org>,
 Nathan Chancellor <nathan@...nel.org>, Peter Zijlstra
 <peterz@...radead.org>, Hao Luo <haoluo@...gle.com>,
 Andrey Konovalov <andreyknvl@...il.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Masahiro Yamada <masahiroy@...nel.org>, Nicolas Schier <nicolas@...sle.eu>,
 Nick Desaulniers <ndesaulniers@...gle.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 linux-kernel@...r.kernel.org, kasan-dev@...glegroups.com,
 linux-hardening@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH v3] ubsan: Reintroduce signed overflow sanitizer



On 2/5/24 10:37, Kees Cook wrote:

> ---
>  include/linux/compiler_types.h |  9 ++++-
>  lib/Kconfig.ubsan              | 14 +++++++
>  lib/test_ubsan.c               | 37 ++++++++++++++++++
>  lib/ubsan.c                    | 68 ++++++++++++++++++++++++++++++++++
>  lib/ubsan.h                    |  4 ++
>  scripts/Makefile.lib           |  3 ++
>  scripts/Makefile.ubsan         |  3 ++
>  7 files changed, 137 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 6f1ca49306d2..ee9d272008a5 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -282,11 +282,18 @@ struct ftrace_likely_data {
>  #define __no_sanitize_or_inline __always_inline
>  #endif
>  
> +/* Do not trap wrapping arithmetic within an annotated function. */
> +#ifdef CONFIG_UBSAN_SIGNED_WRAP
> +# define __signed_wrap __attribute__((no_sanitize("signed-integer-overflow")))
> +#else
> +# define __signed_wrap
> +#endif
> +
>  /* Section for code which can't be instrumented at all */
>  #define __noinstr_section(section)					\
>  	noinline notrace __attribute((__section__(section)))		\
>  	__no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \
> -	__no_sanitize_memory
> +	__no_sanitize_memory __signed_wrap
>  

Given this disables all kinds of code instrumentations,
shouldn't we just add __no_sanitize_undefined here?

I suspect that ubsan's instrumentation usually doesn't cause problems
because it calls __ubsan_* functions with all heavy stuff (printk, locks etc)
only if code has an UB. So the answer to the question above depends on
whether we want to ignore UBs in "noinstr" code or to get some weird side effect,
possibly without proper UBSAN report in dmesg.