[PATCH v2] selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config

[PATCH v2] selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config

[Ricardo Cañuelo]

The lkdtm selftest config fragment enables CONFIG_UBSAN_TRAP to make the
ARRAY_BOUNDS test kill the calling process when an out-of-bound access
is detected by UBSAN. However, after this [1] commit, UBSAN is triggered
under many new scenarios that weren't detected before, such as in struct
definitions with fixed-size trailing arrays used as flexible arrays. As
a result, CONFIG_UBSAN_TRAP=y has become a very aggressive option to
enable except for specific situations.

`make kselftest-merge` applies CONFIG_UBSAN_TRAP=y to the kernel config
for all selftests, which makes many of them fail because of system hangs
during boot.

This change removes the config option from the lkdtm kselftest and
configures the ARRAY_BOUNDS test to look for UBSAN reports rather than
relying on the calling process being killed.

[1] commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC")'

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
---

Changelog:

v2:
  - Configure the ARRAY_BOUNDS lkdtm test to match UBSAN reports instead
    of disabling the test

 tools/testing/selftests/lkdtm/config    | 1 -
 tools/testing/selftests/lkdtm/tests.txt | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config
index 5d52f64dfb43..7afe05e8c4d7 100644
--- a/tools/testing/selftests/lkdtm/config
+++ b/tools/testing/selftests/lkdtm/config
@@ -9,7 +9,6 @@ CONFIG_INIT_ON_FREE_DEFAULT_ON=y
 CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y
 CONFIG_UBSAN=y
 CONFIG_UBSAN_BOUNDS=y
-CONFIG_UBSAN_TRAP=y
 CONFIG_STACKPROTECTOR_STRONG=y
 CONFIG_SLUB_DEBUG=y
 CONFIG_SLUB_DEBUG_ON=y
diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
index 607b8d7e3ea3..2f3a1b96da6e 100644
--- a/tools/testing/selftests/lkdtm/tests.txt
+++ b/tools/testing/selftests/lkdtm/tests.txt
@@ -7,7 +7,7 @@ EXCEPTION
 #EXHAUST_STACK Corrupts memory on failure
 #CORRUPT_STACK Crashes entire system on success
 #CORRUPT_STACK_STRONG Crashes entire system on success
-ARRAY_BOUNDS
+ARRAY_BOUNDS call trace:|UBSAN: array-index-out-of-bounds
 CORRUPT_LIST_ADD list_add corruption
 CORRUPT_LIST_DEL list_del corruption
 STACK_GUARD_PAGE_LEADING
-- 
2.25.1

Re: [PATCH v2] selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config

[Ricardo Cañuelo]

Hi Shuah,

Gentle ping for this, what's the merge status?

On mié, ago 02 2023 at 08:32:52, Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote:
> The lkdtm selftest config fragment enables CONFIG_UBSAN_TRAP to make the
> ARRAY_BOUNDS test kill the calling process when an out-of-bound access
> is detected by UBSAN. However, after this [1] commit, UBSAN is triggered
> under many new scenarios that weren't detected before, such as in struct
> definitions with fixed-size trailing arrays used as flexible arrays. As
> a result, CONFIG_UBSAN_TRAP=y has become a very aggressive option to
> enable except for specific situations.
>
> `make kselftest-merge` applies CONFIG_UBSAN_TRAP=y to the kernel config
> for all selftests, which makes many of them fail because of system hangs
> during boot.
>
> This change removes the config option from the lkdtm kselftest and
> configures the ARRAY_BOUNDS test to look for UBSAN reports rather than
> relying on the calling process being killed.
>
> [1] commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC")'
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
>
> Changelog:
>
> v2:
>   - Configure the ARRAY_BOUNDS lkdtm test to match UBSAN reports instead
>     of disabling the test
>
>  tools/testing/selftests/lkdtm/config    | 1 -
>  tools/testing/selftests/lkdtm/tests.txt | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/lkdtm/config b/tools/testing/selftests/lkdtm/config
> index 5d52f64dfb43..7afe05e8c4d7 100644
> --- a/tools/testing/selftests/lkdtm/config
> +++ b/tools/testing/selftests/lkdtm/config
> @@ -9,7 +9,6 @@ CONFIG_INIT_ON_FREE_DEFAULT_ON=y
>  CONFIG_INIT_ON_ALLOC_DEFAULT_ON=y
>  CONFIG_UBSAN=y
>  CONFIG_UBSAN_BOUNDS=y
> -CONFIG_UBSAN_TRAP=y
>  CONFIG_STACKPROTECTOR_STRONG=y
>  CONFIG_SLUB_DEBUG=y
>  CONFIG_SLUB_DEBUG_ON=y
> diff --git a/tools/testing/selftests/lkdtm/tests.txt b/tools/testing/selftests/lkdtm/tests.txt
> index 607b8d7e3ea3..2f3a1b96da6e 100644
> --- a/tools/testing/selftests/lkdtm/tests.txt
> +++ b/tools/testing/selftests/lkdtm/tests.txt
> @@ -7,7 +7,7 @@ EXCEPTION
>  #EXHAUST_STACK Corrupts memory on failure
>  #CORRUPT_STACK Crashes entire system on success
>  #CORRUPT_STACK_STRONG Crashes entire system on success
> -ARRAY_BOUNDS
> +ARRAY_BOUNDS call trace:|UBSAN: array-index-out-of-bounds
>  CORRUPT_LIST_ADD list_add corruption
>  CORRUPT_LIST_DEL list_del corruption
>  STACK_GUARD_PAGE_LEADING
> -- 
> 2.25.1

Thanks,
Ricardo

Re: [PATCH v2] selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config

[Kees Cook]

On Wed, 02 Aug 2023 08:32:52 +0200, Ricardo Cañuelo wrote:
> The lkdtm selftest config fragment enables CONFIG_UBSAN_TRAP to make the
> ARRAY_BOUNDS test kill the calling process when an out-of-bound access
> is detected by UBSAN. However, after this [1] commit, UBSAN is triggered
> under many new scenarios that weren't detected before, such as in struct
> definitions with fixed-size trailing arrays used as flexible arrays. As
> a result, CONFIG_UBSAN_TRAP=y has become a very aggressive option to
> enable except for specific situations.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config
      https://git.kernel.org/kees/c/64d0436d8588

Take care,

-- 
Kees Cook