Commit 6aaa31ae authored by Andrey Ryabinin's avatar Andrey Ryabinin Committed by Linus Torvalds
Browse files

ubsan: remove overflow checks 

Since GCC 8.0 -fsanitize=signed-integer-overflow doesn't work with
-fwrapv.  -fwrapv makes signed overflows defines and GCC essentially
disables ubsan checks.  On GCC < 8.0 -fwrapv doesn't have influence on
-fsanitize=signed-integer-overflow setting, so it kinda works but
generates false-positves and violates uaccess rules:

lib/iov_iter.o: warning: objtool: iovec_from_user()+0x22d: call to
__ubsan_handle_add_overflow() with UACCESS enabled

Disable signed overflow checks to avoid these problems.  Remove unsigned
overflow checks as well.  Unsigned overflow appeared as side effect of
commit cdf8a76f ("ubsan: move cc-option tests into Kconfig"), but it
never worked (kernel doesn't boot).  And unsigned overflows are allowed by
C standard, so it just pointless.

Link: https://lkml.kernel.org/r/20210209232348.20510-1-ryabinin.a.a@gmail.com


Signed-off-by: default avatarAndrey Ryabinin <ryabinin.a.a@gmail.com>
Acked-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent d54ce615

lib/Kconfig.ubsan

+0 −17
Original line number Diff line number Diff line
@@ -112,23 +112,6 @@ config UBSAN_UNREACHABLE 
	  This option enables -fsanitize=unreachable which checks for control

	  flow reaching an expected-to-be-unreachable position.



config UBSAN_SIGNED_OVERFLOW

	bool "Perform checking for signed arithmetic overflow"

	default UBSAN

	depends on $(cc-option,-fsanitize=signed-integer-overflow)

	help

	  This option enables -fsanitize=signed-integer-overflow which checks

	  for overflow of any arithmetic operations with signed integers.



config UBSAN_UNSIGNED_OVERFLOW

	bool "Perform checking for unsigned arithmetic overflow"

	depends on $(cc-option,-fsanitize=unsigned-integer-overflow)

	depends on !X86_32 # avoid excessive stack usage on x86-32/clang

	help

	  This option enables -fsanitize=unsigned-integer-overflow which checks

	  for overflow of any arithmetic operations with unsigned integers. This

	  currently causes x86 to fail to boot.



config UBSAN_OBJECT_SIZE

	bool "Perform checking for accesses beyond the end of objects"

	default UBSAN

lib/test_ubsan.c

+0 −49
Original line number Diff line number Diff line
@@ -11,51 +11,6 @@ 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;

	volatile unsigned int uval = UINT_MAX;



	UBSAN_TEST(CONFIG_UBSAN_SIGNED_OVERFLOW);

	val += 2;



	UBSAN_TEST(CONFIG_UBSAN_UNSIGNED_OVERFLOW);

	uval += 2;

}



static void test_ubsan_sub_overflow(void)

{

	volatile int val = INT_MIN;

	volatile unsigned int uval = 0;

	volatile int val2 = 2;



	UBSAN_TEST(CONFIG_UBSAN_SIGNED_OVERFLOW);

	val -= val2;



	UBSAN_TEST(CONFIG_UBSAN_UNSIGNED_OVERFLOW);

	uval -= val2;

}



static void test_ubsan_mul_overflow(void)

{

	volatile int val = INT_MAX / 2;

	volatile unsigned int uval = UINT_MAX / 2;



	UBSAN_TEST(CONFIG_UBSAN_SIGNED_OVERFLOW);

	val *= 3;



	UBSAN_TEST(CONFIG_UBSAN_UNSIGNED_OVERFLOW);

	uval *= 3;

}



static void test_ubsan_negate_overflow(void)

{

	volatile int val = INT_MIN;



	UBSAN_TEST(CONFIG_UBSAN_SIGNED_OVERFLOW);

	val = -val;

}



static void test_ubsan_divrem_overflow(void)

{

	volatile int val = 16;
@@ -155,10 +110,6 @@ static void test_ubsan_object_size_mismatch(void) 
}



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,

lib/ubsan.c

+0 −68
Original line number Diff line number Diff line
@@ -163,74 +163,6 @@ static void ubsan_epilogue(void) 
	}

}



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;

scripts/Makefile.ubsan

+0 −2
Original line number Diff line number Diff line
@@ -8,8 +8,6 @@ ubsan-cflags-$(CONFIG_UBSAN_LOCAL_BOUNDS) += -fsanitize=local-bounds 
ubsan-cflags-$(CONFIG_UBSAN_SHIFT)		+= -fsanitize=shift

ubsan-cflags-$(CONFIG_UBSAN_DIV_ZERO)		+= -fsanitize=integer-divide-by-zero

ubsan-cflags-$(CONFIG_UBSAN_UNREACHABLE)	+= -fsanitize=unreachable

ubsan-cflags-$(CONFIG_UBSAN_SIGNED_OVERFLOW)	+= -fsanitize=signed-integer-overflow

ubsan-cflags-$(CONFIG_UBSAN_UNSIGNED_OVERFLOW)	+= -fsanitize=unsigned-integer-overflow

ubsan-cflags-$(CONFIG_UBSAN_OBJECT_SIZE)	+= -fsanitize=object-size

ubsan-cflags-$(CONFIG_UBSAN_BOOL)		+= -fsanitize=bool

ubsan-cflags-$(CONFIG_UBSAN_ENUM)		+= -fsanitize=enum