ubsan: Reintroduce signed overflow sanitizer

In order to mitigate unexpected signed wrap-around[1], bring back the
signed integer overflow sanitizer. It was removed in commit 6aaa31aeb9
("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 more accurately describe the
behavior.

To annotate intentional wrap-around arithmetic, the helpers
wrapping_add/sub/mul_wrap() 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_SIGNED_WRAP_target.o := n"
to mark it as wrapping, and for an entire directory, "UBSAN_SIGNED_WRAP :=
n" can be used.

Additionally keep these disabled under CONFIG_COMPILE_TEST for now.

Link: https://github.com/KSPP/linux/issues/26 [1]
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Hao Luo <haoluo@google.com>
Reviewed-by: Marco Elver <elver@google.com>
Reviewed-by: Justin Stitt <justinstitt@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
This commit is contained in:
Kees Cook 2024-01-18 15:06:05 -08:00
parent 918327e9b7
commit 557f8c582a
7 changed files with 137 additions and 2 deletions

View File

@ -282,11 +282,18 @@ struct ftrace_likely_data {
#define __no_sanitize_or_inline __always_inline #define __no_sanitize_or_inline __always_inline
#endif #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 */ /* Section for code which can't be instrumented at all */
#define __noinstr_section(section) \ #define __noinstr_section(section) \
noinline notrace __attribute((__section__(section))) \ noinline notrace __attribute((__section__(section))) \
__no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \ __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \
__no_sanitize_memory __no_sanitize_memory __signed_wrap
#define noinstr __noinstr_section(".noinstr.text") #define noinstr __noinstr_section(".noinstr.text")

View File

@ -87,7 +87,6 @@ config UBSAN_LOCAL_BOUNDS
config UBSAN_SHIFT config UBSAN_SHIFT
bool "Perform checking for bit-shift overflows" bool "Perform checking for bit-shift overflows"
default UBSAN
depends on $(cc-option,-fsanitize=shift) depends on $(cc-option,-fsanitize=shift)
help help
This option enables -fsanitize=shift which checks for bit-shift This option enables -fsanitize=shift which checks for bit-shift
@ -116,6 +115,20 @@ config UBSAN_UNREACHABLE
This option enables -fsanitize=unreachable which checks for control This option enables -fsanitize=unreachable which checks for control
flow reaching an expected-to-be-unreachable position. 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 config UBSAN_BOOL
bool "Perform checking for non-boolean values used as boolean" bool "Perform checking for non-boolean values used as boolean"
default UBSAN default UBSAN

View File

@ -11,6 +11,39 @@ typedef void(*test_ubsan_fp)(void);
#config, IS_ENABLED(config) ? "y" : "n"); \ #config, IS_ENABLED(config) ? "y" : "n"); \
} while (0) } 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) static void test_ubsan_divrem_overflow(void)
{ {
volatile int val = 16; volatile int val = 16;
@ -90,6 +123,10 @@ static void test_ubsan_misaligned_access(void)
} }
static const test_ubsan_fp test_ubsan_array[] = { 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_shift_out_of_bounds,
test_ubsan_out_of_bounds, test_ubsan_out_of_bounds,
test_ubsan_load_invalid_value, test_ubsan_load_invalid_value,

View File

@ -222,6 +222,74 @@ static void ubsan_epilogue(void)
check_panic_on_warn("UBSAN"); 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) void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs)
{ {
struct overflow_data *data = _data; struct overflow_data *data = _data;

View File

@ -124,6 +124,10 @@ typedef s64 s_max;
typedef u64 u_max; typedef u64 u_max;
#endif #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_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(struct type_mismatch_data *data, void *ptr);
void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr); void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr);

View File

@ -177,6 +177,9 @@ ifeq ($(CONFIG_UBSAN),y)
_c_flags += $(if $(patsubst n%,, \ _c_flags += $(if $(patsubst n%,, \
$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)y), \ $(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)y), \
$(CFLAGS_UBSAN)) $(CFLAGS_UBSAN))
_c_flags += $(if $(patsubst n%,, \
$(UBSAN_SIGNED_WRAP_$(basetarget).o)$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SIGNED_WRAP)$(UBSAN_SANITIZE)y), \
$(CFLAGS_UBSAN_SIGNED_WRAP))
endif endif
ifeq ($(CONFIG_KCOV),y) ifeq ($(CONFIG_KCOV),y)

View File

@ -13,3 +13,6 @@ ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -fsanitize=enum
ubsan-cflags-$(CONFIG_UBSAN_TRAP) += $(call cc-option,-fsanitize-trap=undefined,-fsanitize-undefined-trap-on-error) ubsan-cflags-$(CONFIG_UBSAN_TRAP) += $(call cc-option,-fsanitize-trap=undefined,-fsanitize-undefined-trap-on-error)
export CFLAGS_UBSAN := $(ubsan-cflags-y) export CFLAGS_UBSAN := $(ubsan-cflags-y)
ubsan-signed-wrap-cflags-$(CONFIG_UBSAN_SIGNED_WRAP) += -fsanitize=signed-integer-overflow
export CFLAGS_UBSAN_SIGNED_WRAP := $(ubsan-signed-wrap-cflags-y)