From 6dde3569b867e2af2a9576c2f3ca1aa9b87d39fd Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Mon, 22 Jan 2024 16:49:35 -0800 Subject: lkdtm/bugs: Adjust lkdtm_HUNG_TASK() to avoid tail call optimization When testing with lkdtm_HUNG_TASK() and looking at the output, I expected to see lkdtm_HUNG_TASK() in the stack crawl but it wasn't there. Instead, the top function on at least some devices was schedule() due to tail call optimization. Let's do two things to help here: 1. We'll mark this as "__noreturn". On GCC at least this is documented to prevent tail call optimization. The docs [1] say "In order to preserve backtraces, GCC will never turn calls to noreturn functions into tail calls." 2. We'll add a BUG_ON(1) at the end which means that schedule() is no longer a tail call. Note that this is potentially important because if we _did_ end up returning from schedule() due to some weird issue then we'd potentially be violating the "noreturn" that we told the compiler about. BUG is the right thing to do here. [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20240122164935.2.I26e8f68c312824fcc80c19d4e91de2d2bef958f0@changeid Signed-off-by: Kees Cook --- drivers/misc/lkdtm/bugs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/misc/lkdtm/bugs.c') diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index b080eb2335eb..d1222d3eda2f 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -294,10 +294,11 @@ static void lkdtm_SPINLOCKUP(void) __release(&lock_me_up); } -static void lkdtm_HUNG_TASK(void) +static void __noreturn lkdtm_HUNG_TASK(void) { set_current_state(TASK_UNINTERRUPTIBLE); schedule(); + BUG_ON(1); } static volatile unsigned int huge = INT_MAX - 2; -- cgit v1.2.3 From 735b7636d1a88e85eeef607a8179a114618bc5a0 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Fri, 26 Jan 2024 07:28:53 -0800 Subject: lkdtm/bugs: In lkdtm_HUNG_TASK() use BUG(), not BUG_ON(1) In commit edb6538da3df ("lkdtm/bugs: Adjust lkdtm_HUNG_TASK() to avoid tail call optimization") we marked lkdtm_HUNG_TASK() as __noreturn. The compiler gets unhappy if it thinks a __noreturn function might return, so there's a BUG_ON(1) at the end. Any human can see that the function won't return and the compiler can figure that out too. Except when it can't. The MIPS architecture defines HAVE_ARCH_BUG_ON and defines its own version of BUG_ON(). The MIPS version of BUG_ON() is not a macro but is instead an inline function. Apparently this prevents the compiler from realizing that the condition to BUG_ON() is constant and that the function will never return. Let's change the BUG_ON(1) to just BUG(), which it should have been to begin with. The only reason I used BUG_ON(1) to begin with was because I was used to using WARN_ON(1) when writing test code and WARN() and BUG() are oddly inconsistent in this manner. :-/ Fixes: edb6538da3df ("lkdtm/bugs: Adjust lkdtm_HUNG_TASK() to avoid tail call optimization") Signed-off-by: Douglas Anderson Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202401262204.wUFKRYZF-lkp@intel.com/ Acked-by: Arnd Bergmann Link: https://lore.kernel.org/r/20240126072852.1.Ib065e528a8620474a72f15baa2feead1f3d89865@changeid Signed-off-by: Kees Cook --- drivers/misc/lkdtm/bugs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/misc/lkdtm/bugs.c') diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index d1222d3eda2f..b92767d6bdd2 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -298,7 +298,7 @@ static void __noreturn lkdtm_HUNG_TASK(void) { set_current_state(TASK_UNINTERRUPTIBLE); schedule(); - BUG_ON(1); + BUG(); } static volatile unsigned int huge = INT_MAX - 2; -- cgit v1.2.3 From 231dc3f0c936db142ef3fa922f1ab751dd532d70 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Thu, 21 Mar 2024 13:18:17 -0700 Subject: lkdtm/bugs: Improve warning message for compilers without counted_by support The current message for telling the user that their compiler does not support the counted_by attribute in the FAM_BOUNDS test does not make much sense either grammatically or semantically. Fix it to make it correct in both aspects. Signed-off-by: Nathan Chancellor Reviewed-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/20240321-lkdtm-improve-lack-of-counted_by-msg-v1-1-0fbf7481a29c@kernel.org Signed-off-by: Kees Cook --- drivers/misc/lkdtm/bugs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/misc/lkdtm/bugs.c') diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c index b92767d6bdd2..5178c02b21eb 100644 --- a/drivers/misc/lkdtm/bugs.c +++ b/drivers/misc/lkdtm/bugs.c @@ -417,7 +417,7 @@ static void lkdtm_FAM_BOUNDS(void) pr_err("FAIL: survived access of invalid flexible array member index!\n"); if (!__has_attribute(__counted_by__)) - pr_warn("This is expected since this %s was built a compiler supporting __counted_by\n", + pr_warn("This is expected since this %s was built with a compiler that does not support __counted_by\n", lkdtm_kernel_info); else if (IS_ENABLED(CONFIG_UBSAN_BOUNDS)) pr_expected_config(CONFIG_UBSAN_TRAP); -- cgit v1.2.3