diff mbox series

ubsan: Remove CONFIG_UBSAN_OBJECT_SIZE

Message ID 20211203235346.110809-1-keescook@chromium.org (mailing list archive)
State Mainlined
Commit 69d0db01e210e07fe915e5da91b54a867cda040f
Delegated to: Kees Cook
Headers show
Series ubsan: Remove CONFIG_UBSAN_OBJECT_SIZE | expand

Commit Message

Kees Cook Dec. 3, 2021, 11:53 p.m. UTC
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(-)

Comments

Marco Elver Dec. 6, 2021, 7 a.m. UTC | #1
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 mbox series

Patch

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