On Wed, Feb 21, 2024 at 04:03:07PM -0800, Andrew Morton wrote: > > The quilt patch titled > Subject: ubsan: reintroduce signed overflow sanitizer > has been removed from the -mm tree. Its filename was > ubsan-reintroduce-signed-overflow-sanitizer.patch Hi Andrew, Please drop this -- it has several prerequisites, and I'm already carrying it in the hardening tree (since that's where UBSAN is carried now[1]). Thanks! -Kees [1] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/kspp&id=0ea74b4de34a12396fe3790590007aa50fcb5d45 > > This patch was dropped because it was merged into the mm-stable branch > of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > > ------------------------------------------------------ > From: Kees Cook <keescook@xxxxxxxxxxxx> > Subject: ubsan: reintroduce signed overflow sanitizer > Date: Mon, 5 Feb 2024 01:37:29 -0800 > > In order to mitigate unexpected signed wrap-around[1], bring back the > signed integer overflow sanitizer. It was removed in commit 6aaa31aeb9cf > ("ubsan: remove overflow checks") because it was effectively a no-op when > combined with -fno-strict-overflow (which correctly changes signed > overflow from being "undefined" to being explicitly "wrap around"). > > Compilers are adjusting their sanitizers to trap wrap-around and to > detecting common code patterns that should not be instrumented (e.g. "var > + offset < var"). Prepare for this and explicitly rename the option from > "OVERFLOW" to "WRAP". > > To annotate intentional wrap-around arithmetic, the add/sub/mul_wrap() > helpers can be used for individual statements. At the function level, the > __signed_wrap attribute can be used to mark an entire function as > expecting its signed arithmetic to wrap around. For a single object file > the Makefile can use "UBSAN_WRAP_SIGNED_target.o := n" to mark it as > wrapping, and for an entire directory, "UBSAN_WRAP_SIGNED := n" can be > used. > > Additionally keep these disabled under CONFIG_COMPILE_TEST for now. > > Link: https://github.com/KSPP/linux/issues/26 [1] > Link: https://lkml.kernel.org/r/20240205093725.make.582-kees@xxxxxxxxxx > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > Reviewed-by: Marco Elver <elver@xxxxxxxxxx> > Cc: Justin Stitt <justinstitt@xxxxxxxxxx> > Cc: Miguel Ojeda <ojeda@xxxxxxxxxx> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Hao Luo <haoluo@xxxxxxxxxx> > Cc: Andrey Konovalov <andreyknvl@xxxxxxxxx> > Cc: Andrey Ryabinin <ryabinin.a.a@xxxxxxxxx> > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Cc: Nicolas Schier <nicolas@xxxxxxxxx> > Cc: Przemek Kitszel <przemyslaw.kitszel@xxxxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > --- > > 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(-) > > --- a/include/linux/compiler_types.h~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/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 > > #define noinstr __noinstr_section(".noinstr.text") > > --- a/lib/Kconfig.ubsan~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/lib/Kconfig.ubsan > @@ -116,6 +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" > + default UBSAN > + depends on !COMPILE_TEST > + 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). > + > config UBSAN_BOOL > bool "Perform checking for non-boolean values used as boolean" > default UBSAN > --- a/lib/test_ubsan.c~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/lib/test_ubsan.c > @@ -11,6 +11,39 @@ typedef void(*test_ubsan_fp)(void); > #config, IS_ENABLED(config) ? "y" : "n"); \ > } while (0) > > +static void test_ubsan_add_overflow(void) > +{ > + volatile int val = INT_MAX; > + > + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); > + val += 2; > +} > + > +static void test_ubsan_sub_overflow(void) > +{ > + volatile int val = INT_MIN; > + volatile int val2 = 2; > + > + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); > + val -= val2; > +} > + > +static void test_ubsan_mul_overflow(void) > +{ > + volatile int val = INT_MAX / 2; > + > + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); > + val *= 3; > +} > + > +static void test_ubsan_negate_overflow(void) > +{ > + volatile int val = INT_MIN; > + > + UBSAN_TEST(CONFIG_UBSAN_SIGNED_WRAP); > + val = -val; > +} > + > static void test_ubsan_divrem_overflow(void) > { > volatile int val = 16; > @@ -90,6 +123,10 @@ static void test_ubsan_misaligned_access > } > > static const test_ubsan_fp test_ubsan_array[] = { > + test_ubsan_add_overflow, > + test_ubsan_sub_overflow, > + test_ubsan_mul_overflow, > + test_ubsan_negate_overflow, > test_ubsan_shift_out_of_bounds, > test_ubsan_out_of_bounds, > test_ubsan_load_invalid_value, > --- a/lib/ubsan.c~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/lib/ubsan.c > @@ -222,6 +222,74 @@ static void ubsan_epilogue(void) > check_panic_on_warn("UBSAN"); > } > > +static void handle_overflow(struct overflow_data *data, void *lhs, > + void *rhs, char op) > +{ > + > + struct type_descriptor *type = data->type; > + char lhs_val_str[VALUE_LENGTH]; > + char rhs_val_str[VALUE_LENGTH]; > + > + if (suppress_report(&data->location)) > + return; > + > + ubsan_prologue(&data->location, type_is_signed(type) ? > + "signed-integer-overflow" : > + "unsigned-integer-overflow"); > + > + val_to_string(lhs_val_str, sizeof(lhs_val_str), type, lhs); > + val_to_string(rhs_val_str, sizeof(rhs_val_str), type, rhs); > + pr_err("%s %c %s cannot be represented in type %s\n", > + lhs_val_str, > + op, > + rhs_val_str, > + type->type_name); > + > + ubsan_epilogue(); > +} > + > +void __ubsan_handle_add_overflow(void *data, > + void *lhs, void *rhs) > +{ > + > + handle_overflow(data, lhs, rhs, '+'); > +} > +EXPORT_SYMBOL(__ubsan_handle_add_overflow); > + > +void __ubsan_handle_sub_overflow(void *data, > + void *lhs, void *rhs) > +{ > + handle_overflow(data, lhs, rhs, '-'); > +} > +EXPORT_SYMBOL(__ubsan_handle_sub_overflow); > + > +void __ubsan_handle_mul_overflow(void *data, > + void *lhs, void *rhs) > +{ > + handle_overflow(data, lhs, rhs, '*'); > +} > +EXPORT_SYMBOL(__ubsan_handle_mul_overflow); > + > +void __ubsan_handle_negate_overflow(void *_data, void *old_val) > +{ > + struct overflow_data *data = _data; > + char old_val_str[VALUE_LENGTH]; > + > + if (suppress_report(&data->location)) > + return; > + > + ubsan_prologue(&data->location, "negation-overflow"); > + > + val_to_string(old_val_str, sizeof(old_val_str), data->type, old_val); > + > + pr_err("negation of %s cannot be represented in type %s:\n", > + old_val_str, data->type->type_name); > + > + ubsan_epilogue(); > +} > +EXPORT_SYMBOL(__ubsan_handle_negate_overflow); > + > + > void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs) > { > struct overflow_data *data = _data; > --- a/lib/ubsan.h~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/lib/ubsan.h > @@ -124,6 +124,10 @@ typedef s64 s_max; > typedef u64 u_max; > #endif > > +void __ubsan_handle_add_overflow(void *data, void *lhs, void *rhs); > +void __ubsan_handle_sub_overflow(void *data, void *lhs, void *rhs); > +void __ubsan_handle_mul_overflow(void *data, void *lhs, void *rhs); > +void __ubsan_handle_negate_overflow(void *_data, void *old_val); > void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs); > void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr); > void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr); > --- a/scripts/Makefile.lib~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/scripts/Makefile.lib > @@ -177,6 +177,9 @@ ifeq ($(CONFIG_UBSAN),y) > _c_flags += $(if $(patsubst n%,, \ > $(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)$(CONFIG_UBSAN_SANITIZE_ALL)), \ > $(CFLAGS_UBSAN)) > +_c_flags += $(if $(patsubst n%,, \ > + $(UBSAN_WRAP_SIGNED_$(basetarget).o)$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_WRAP_SIGNED)$(UBSAN_SANITIZE)y), \ > + $(CFLAGS_UBSAN_WRAP_SIGNED)) > endif > > ifeq ($(CONFIG_KCOV),y) > --- a/scripts/Makefile.ubsan~ubsan-reintroduce-signed-overflow-sanitizer > +++ a/scripts/Makefile.ubsan > @@ -13,3 +13,6 @@ ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -f > ubsan-cflags-$(CONFIG_UBSAN_TRAP) += -fsanitize-undefined-trap-on-error > > export CFLAGS_UBSAN := $(ubsan-cflags-y) > + > +ubsan-wrap-signed-cflags-$(CONFIG_UBSAN_SIGNED_WRAP) += -fsanitize=signed-integer-overflow > +export CFLAGS_UBSAN_WRAP_SIGNED := $(ubsan-wrap-signed-cflags-y) > _ > > Patches currently in -mm which might be from keescook@xxxxxxxxxxxx are > > -- Kees Cook
