| Message ID | 20211203235346.110809-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ubsan: Remove CONFIG_UBSAN_OBJECT_SIZE | expand |
On Sat, 4 Dec 2021 at 00:53, Kees Cook <keescook@chromium.org> wrote: > The object-size sanitizer is redundant to -Warray-bounds, and > inappropriately performs its checks at run-time when all information > needed for the evaluation is available at compile-time, making it quite > difficult to use: > > https://bugzilla.kernel.org/show_bug.cgi?id=214861 > > With -Warray-bounds almost enabled globally, it doesn't make sense to > keep this around. > > Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: Marco Elver <elver@google.com> Thank you! > --- > lib/Kconfig.ubsan | 13 ------------- > lib/test_ubsan.c | 22 ---------------------- > scripts/Makefile.ubsan | 1 - > 3 files changed, 36 deletions(-) > > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan > index e5372a13511d..236c5cefc4cc 100644 > --- a/lib/Kconfig.ubsan > +++ b/lib/Kconfig.ubsan > @@ -112,19 +112,6 @@ config UBSAN_UNREACHABLE > This option enables -fsanitize=unreachable which checks for control > flow reaching an expected-to-be-unreachable position. > > -config UBSAN_OBJECT_SIZE > - bool "Perform checking for accesses beyond the end of objects" > - default UBSAN > - # gcc hugely expands stack usage with -fsanitize=object-size > - # https://lore.kernel.org/lkml/CAHk-=wjPasyJrDuwDnpHJS2TuQfExwe=px-SzLeN8GFMAQJPmQ@mail.gmail.com/ > - depends on !CC_IS_GCC > - depends on $(cc-option,-fsanitize=object-size) > - help > - This option enables -fsanitize=object-size which checks for accesses > - beyond the end of objects where the optimizer can determine both the > - object being operated on and its size, usually seen with bad downcasts, > - or access to struct members from NULL pointers. > - > config UBSAN_BOOL > bool "Perform checking for non-boolean values used as boolean" > default UBSAN > diff --git a/lib/test_ubsan.c b/lib/test_ubsan.c > index 7e7bbd0f3fd2..2062be1f2e80 100644 > --- a/lib/test_ubsan.c > +++ b/lib/test_ubsan.c > @@ -79,15 +79,6 @@ static void test_ubsan_load_invalid_value(void) > eval2 = eval; > } > > -static void test_ubsan_null_ptr_deref(void) > -{ > - volatile int *ptr = NULL; > - int val; > - > - UBSAN_TEST(CONFIG_UBSAN_OBJECT_SIZE); > - val = *ptr; > -} > - > static void test_ubsan_misaligned_access(void) > { > volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5}; > @@ -98,29 +89,16 @@ static void test_ubsan_misaligned_access(void) > *ptr = val; > } > > -static void test_ubsan_object_size_mismatch(void) > -{ > - /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */ > - volatile int val __aligned(8) = 4; > - volatile long long *ptr, val2; > - > - UBSAN_TEST(CONFIG_UBSAN_OBJECT_SIZE); > - ptr = (long long *)&val; > - val2 = *ptr; > -} > - > static const test_ubsan_fp test_ubsan_array[] = { > test_ubsan_shift_out_of_bounds, > test_ubsan_out_of_bounds, > test_ubsan_load_invalid_value, > test_ubsan_misaligned_access, > - test_ubsan_object_size_mismatch, > }; > > /* Excluded because they Oops the module. */ > static const test_ubsan_fp skip_ubsan_array[] = { > test_ubsan_divrem_overflow, > - test_ubsan_null_ptr_deref, > }; > > static int __init test_ubsan_init(void) > diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan > index 9e2092fd5206..7099c603ff0a 100644 > --- a/scripts/Makefile.ubsan > +++ b/scripts/Makefile.ubsan > @@ -8,7 +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_OBJECT_SIZE) += -fsanitize=object-size > ubsan-cflags-$(CONFIG_UBSAN_BOOL) += -fsanitize=bool > ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -fsanitize=enum > ubsan-cflags-$(CONFIG_UBSAN_TRAP) += -fsanitize-undefined-trap-on-error > -- > 2.30.2 >
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index e5372a13511d..236c5cefc4cc 100644 --- a/lib/Kconfig.ubsan +++ b/lib/Kconfig.ubsan @@ -112,19 +112,6 @@ config UBSAN_UNREACHABLE This option enables -fsanitize=unreachable which checks for control flow reaching an expected-to-be-unreachable position. -config UBSAN_OBJECT_SIZE - bool "Perform checking for accesses beyond the end of objects" - default UBSAN - # gcc hugely expands stack usage with -fsanitize=object-size - # https://lore.kernel.org/lkml/CAHk-=wjPasyJrDuwDnpHJS2TuQfExwe=px-SzLeN8GFMAQJPmQ@mail.gmail.com/ - depends on !CC_IS_GCC - depends on $(cc-option,-fsanitize=object-size) - help - This option enables -fsanitize=object-size which checks for accesses - beyond the end of objects where the optimizer can determine both the - object being operated on and its size, usually seen with bad downcasts, - or access to struct members from NULL pointers. - config UBSAN_BOOL bool "Perform checking for non-boolean values used as boolean" default UBSAN diff --git a/lib/test_ubsan.c b/lib/test_ubsan.c index 7e7bbd0f3fd2..2062be1f2e80 100644 --- a/lib/test_ubsan.c +++ b/lib/test_ubsan.c @@ -79,15 +79,6 @@ static void test_ubsan_load_invalid_value(void) eval2 = eval; } -static void test_ubsan_null_ptr_deref(void) -{ - volatile int *ptr = NULL; - int val; - - UBSAN_TEST(CONFIG_UBSAN_OBJECT_SIZE); - val = *ptr; -} - static void test_ubsan_misaligned_access(void) { volatile char arr[5] __aligned(4) = {1, 2, 3, 4, 5}; @@ -98,29 +89,16 @@ static void test_ubsan_misaligned_access(void) *ptr = val; } -static void test_ubsan_object_size_mismatch(void) -{ - /* "((aligned(8)))" helps this not into be misaligned for ptr-access. */ - volatile int val __aligned(8) = 4; - volatile long long *ptr, val2; - - UBSAN_TEST(CONFIG_UBSAN_OBJECT_SIZE); - ptr = (long long *)&val; - val2 = *ptr; -} - static const test_ubsan_fp test_ubsan_array[] = { test_ubsan_shift_out_of_bounds, test_ubsan_out_of_bounds, test_ubsan_load_invalid_value, test_ubsan_misaligned_access, - test_ubsan_object_size_mismatch, }; /* Excluded because they Oops the module. */ static const test_ubsan_fp skip_ubsan_array[] = { test_ubsan_divrem_overflow, - test_ubsan_null_ptr_deref, }; static int __init test_ubsan_init(void) diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan index 9e2092fd5206..7099c603ff0a 100644 --- a/scripts/Makefile.ubsan +++ b/scripts/Makefile.ubsan @@ -8,7 +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_OBJECT_SIZE) += -fsanitize=object-size ubsan-cflags-$(CONFIG_UBSAN_BOOL) += -fsanitize=bool ubsan-cflags-$(CONFIG_UBSAN_ENUM) += -fsanitize=enum ubsan-cflags-$(CONFIG_UBSAN_TRAP) += -fsanitize-undefined-trap-on-error
The object-size sanitizer is redundant to -Warray-bounds, and inappropriately performs its checks at run-time when all information needed for the evaluation is available at compile-time, making it quite difficult to use: https://bugzilla.kernel.org/show_bug.cgi?id=214861 With -Warray-bounds almost enabled globally, it doesn't make sense to keep this around. Signed-off-by: Kees Cook <keescook@chromium.org> --- lib/Kconfig.ubsan | 13 ------------- lib/test_ubsan.c | 22 ---------------------- scripts/Makefile.ubsan | 1 - 3 files changed, 36 deletions(-)