[PATCH] lkdtm/stackleak: fix CONFIG_GCC_PLUGIN_STACKLEAK=n

[PATCH] lkdtm/stackleak: fix CONFIG_GCC_PLUGIN_STACKLEAK=n

[Mark Rutland]

Recent rework broke building LKDTM when CONFIG_GCC_PLUGIN_STACKLEAK=n.
This patch fixes that breakage.

Prior to recent stackleak rework, the LKDTM STACKLEAK_ERASING code could
be built when the kernel was not built with stackleak support, and would
run a test that would almost certainly fail (or pass by sheer cosmic
coincidence), e.g.

| # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry STACKLEAK_ERASING
| lkdtm: checking unused part of the thread stack (15560 bytes)...
| lkdtm: FAIL: the erased part is not found (checked 15560 bytes)
| lkdtm: FAIL: the thread stack is NOT properly erased!
| lkdtm: This is probably expected, since this kernel (5.18.0-rc2 aarch64) was built *without* CONFIG_GCC_PLUGIN_STACKLEAK=y

The recent rework to the test made it more accurate by using helpers
which are only defined when CONFIG_GCC_PLUGIN_STACKLEAK=y, and so when
building LKDTM when CONFIG_GCC_PLUGIN_STACKLEAK=n, we get a build
failure:

| drivers/misc/lkdtm/stackleak.c: In function 'check_stackleak_irqoff':
| drivers/misc/lkdtm/stackleak.c:30:46: error: implicit declaration of function 'stackleak_task_low_bound' [-Werror=implicit-function-declaration]
|    30 |         const unsigned long task_stack_low = stackleak_task_low_bound(current);
|       |                                              ^~~~~~~~~~~~~~~~~~~~~~~~
| drivers/misc/lkdtm/stackleak.c:31:47: error: implicit declaration of function 'stackleak_task_high_bound'; did you mean 'stackleak_task_init'? [-Werror=implicit-function-declaration]
|    31 |         const unsigned long task_stack_high = stackleak_task_high_bound(current);
|       |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
|       |                                               stackleak_task_init
| drivers/misc/lkdtm/stackleak.c:33:48: error: 'struct task_struct' has no member named 'lowest_stack'
|    33 |         const unsigned long lowest_sp = current->lowest_stack;
|       |                                                ^~
| drivers/misc/lkdtm/stackleak.c:74:23: error: implicit declaration of function 'stackleak_find_top_of_poison' [-Werror=implicit-function-declaration]
|    74 |         poison_high = stackleak_find_top_of_poison(task_stack_low, untracked_high);
|       |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

This patch fixes the issue by not compiling the body of the test when
CONFIG_GCC_PLUGIN_STACKLEAK=n, and replacing this with an unconditional
XFAIL message. This means the pr_expected_config() in
check_stackleak_irqoff() is redundant, and so it is removed.

Where an architecture does not support stackleak, the test will log:

| # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry STACKLEAK_ERASING
| lkdtm: XFAIL: stackleak is not supported on this arch (HAVE_ARCH_STACKLEAK=n)

Where an architectures does support stackleak, but this has not been
compiled in, the test will log:

| # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry STACKLEAK_ERASING
| lkdtm: XFAIL: stackleak is not enabled (CONFIG_GCC_PLUGIN_STACKLEAK=n)

Where stackleak has been compiled in, the test behaves as usual:

| # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
| lkdtm: Performing direct entry STACKLEAK_ERASING
| lkdtm: stackleak stack usage:
|   high offset: 336 bytes
|   current:     688 bytes
|   lowest:      1232 bytes
|   tracked:     1232 bytes
|   untracked:   672 bytes
|   poisoned:    14136 bytes
|   low offset:  8 bytes
| lkdtm: OK: the rest of the thread stack is properly erased

Fixes: f4cfacd92972cc44 ("lkdtm/stackleak: rework boundary management")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Popov <alex.popov@linux.com>
Cc: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm/stackleak.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm/stackleak.c b/drivers/misc/lkdtm/stackleak.c
index 52800583fd051..82369c6f889e2 100644
--- a/drivers/misc/lkdtm/stackleak.c
+++ b/drivers/misc/lkdtm/stackleak.c
@@ -11,6 +11,7 @@
 #include "lkdtm.h"
 #include <linux/stackleak.h>
 
+#if defined(CONFIG_GCC_PLUGIN_STACKLEAK)
 /*
  * Check that stackleak tracks the lowest stack pointer and erases the stack
  * below this as expected.
@@ -109,7 +110,6 @@ static void noinstr check_stackleak_irqoff(void)
 out:
 	if (test_failed) {
 		pr_err("FAIL: the thread stack is NOT properly erased!\n");
-		pr_expected_config(CONFIG_GCC_PLUGIN_STACKLEAK);
 	} else {
 		pr_info("OK: the rest of the thread stack is properly erased\n");
 	}
@@ -123,3 +123,13 @@ void lkdtm_STACKLEAK_ERASING(void)
 	check_stackleak_irqoff();
 	local_irq_restore(flags);
 }
+#else /* defined(CONFIG_GCC_PLUGIN_STACKLEAK) */
+void lkdtm_STACKLEAK_ERASING(void)
+{
+	if (IS_ENABLED(CONFIG_HAVE_ARCH_STACKLEAK)) {
+		pr_err("XFAIL: stackleak is not enabled (CONFIG_GCC_PLUGIN_STACKLEAK=n)\n");
+	} else {
+		pr_err("XFAIL: stackleak is not supported on this arch (HAVE_ARCH_STACKLEAK=n)\n");
+	}
+}
+#endif /* defined(CONFIG_GCC_PLUGIN_STACKLEAK) */
-- 
2.30.2

Re: [PATCH] lkdtm/stackleak: fix CONFIG_GCC_PLUGIN_STACKLEAK=n

[Kees Cook]

On Fri, 6 May 2022 13:11:45 +0100, Mark Rutland wrote:
> Recent rework broke building LKDTM when CONFIG_GCC_PLUGIN_STACKLEAK=n.
> This patch fixes that breakage.
> 
> Prior to recent stackleak rework, the LKDTM STACKLEAK_ERASING code could
> be built when the kernel was not built with stackleak support, and would
> run a test that would almost certainly fail (or pass by sheer cosmic
> coincidence), e.g.
> 
> [...]

Applied to for-next/hardening, thanks!

[1/1] lkdtm/stackleak: fix CONFIG_GCC_PLUGIN_STACKLEAK=n
      https://git.kernel.org/kees/c/932c12ae7963

-- 
Kees Cook