diff mbox series

selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config

Message ID 20230801094329.1878928-1-ricardo.canuelo@collabora.com (mailing list archive)
State New
Headers show
Series selftests/lkdtm: Disable CONFIG_UBSAN_TRAP in test config | expand

Commit Message

Ricardo Cañuelo Aug. 1, 2023, 9:43 a.m. UTC
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 also
the ARRAY_BOUNDS test to skip it rather than have it failing. If
out-of-bound array accesses need to be checked, there's
CONFIG_TEST_UBSAN for that.

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

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 tools/testing/selftests/lkdtm/config    | 1 -
 tools/testing/selftests/lkdtm/tests.txt | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

Kees Cook Aug. 2, 2023, 12:26 a.m. UTC | #1
On Tue, Aug 01, 2023 at 11:43:29AM +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.

Yeah, that's fair. We need to actually get these issues reported and
TRAP doesn't help with that.

> 
> `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 also
> the ARRAY_BOUNDS test to skip it rather than have it failing. If
> out-of-bound array accesses need to be checked, there's
> CONFIG_TEST_UBSAN for that.

I *think* instead, we can turn off TRAP but retain the ARRAY_BOUNDS
kselftest by looking for either WARN or TRAP results:

-ARRAY_BOUNDS
+ARRAY_BOUNDS call trace:|UBSAN: array-index-out-of-bounds

Can test that and send a v2?

-Kees
Ricardo Cañuelo Aug. 2, 2023, 6:18 a.m. UTC | #2
Hi Kees,

Thanks for reviewing the patch,

On mar, ago 01 2023 at 17:26:44, Kees Cook <keescook@chromium.org> wrote:
> I *think* instead, we can turn off TRAP but retain the ARRAY_BOUNDS
> kselftest by looking for either WARN or TRAP results:
>
> -ARRAY_BOUNDS
> +ARRAY_BOUNDS call trace:|UBSAN: array-index-out-of-bounds
>
> Can test that and send a v2?

Good idea! Sure, I can test it and I'll send v2 in a while.

Cheers,
Ricardo
diff mbox series

Patch

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..6a49f2abbda8 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 Needs CONFIG_UBSAN_TRAP=y, might cause unrelated system hangs
 CORRUPT_LIST_ADD list_add corruption
 CORRUPT_LIST_DEL list_del corruption
 STACK_GUARD_PAGE_LEADING