[PATCH] kmsan: fix false warnings in return_address on s390

[PATCH] kmsan: fix false warnings in return_address on s390

[Aleksei Nikiforov]

Function return_address manually traverses stack frames
using backchain on s390.
Stack frames are written to stack in function prologues
and are not marked as written for kmsan.
This may lead to previous mark being used.

Let's assume that there was an unitialized area on stack.
Later backchain and other information is written there
in function prologue.
kmsan markings are unchanged in prologue.
That means data is actually initialized but incorrectly marked.
And when data is read, a false warning is emitted.

It might be possible to fix marking data written in prologue of function,
but it may be somewhat complex, especially in leaf functions
where no stack for next function call is allocated yet
and new function call would be required for kmsan helper function.

Other approach is to mark return_address function as noinstr
to skip kmsan checks there. That's approach in this patch.

When kmsan is enabled, a special noinstr wrapper function is used.
Since one more function call is introduced,
depth n of backtrace is incremented to account for this function call.

Fix false warnings like following one:

[    1.017151] =====================================================
[    1.017218] BUG: KMSAN: uninit-value in trace_hardirqs_off+0x134/0x5e0
[    1.017280]  trace_hardirqs_off+0x134/0x5e0
[    1.017330]  _raw_spin_lock_irqsave+0x74/0xc0
[    1.017375]  get_from_partial_node+0x112/0x1df0
[    1.017424]  ___slab_alloc+0x27c/0x29a0
[    1.017466]  __kmalloc_noprof+0x442/0x1cb0
[    1.017507]  alloc_workqueue_va+0x188/0x3f30
[    1.017552]  alloc_workqueue_noprof+0x236/0x260
[    1.017595]  kmem_cache_init_late+0x44/0xe0
[    1.017648]  start_kernel+0x57a/0xcd0
[    1.017689]  startup_continue+0x2e/0x40
[    1.017724]
[    1.017732] Local variable old created at:
[    1.017747]  get_from_partial_node+0x35c/0x1df0
[    1.017790]
[    1.017804] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.2.0 #54 PREEMPT
[    1.017859] Hardware name: IBM 8561 T01 703 (KVM/Linux)
[    1.017877] =====================================================

Signed-off-by: Aleksei Nikiforov <aleksei.nikiforov@linux.ibm.com>
---
 arch/s390/include/asm/ftrace.h | 6 ++++++
 arch/s390/kernel/stacktrace.c  | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 692c484ec163..9a9f7f2f2c36 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -23,7 +23,13 @@ static __always_inline unsigned long return_address(unsigned int n)
 	} while (--n);
 	return sf->gprs[8];
 }
+
+#ifdef CONFIG_KMSAN
+unsigned long return_address_noinstr(unsigned int n);
+#define ftrace_return_address(n) return_address_noinstr(n)
+#else
 #define ftrace_return_address(n) return_address(n)
+#endif
 
 void ftrace_caller(void);
 
diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c
index 18520d333058..db458050d8bf 100644
--- a/arch/s390/kernel/stacktrace.c
+++ b/arch/s390/kernel/stacktrace.c
@@ -146,3 +146,11 @@ void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie,
 {
 	arch_stack_walk_user_common(consume_entry, cookie, NULL, regs, false);
 }
+
+#ifdef CONFIG_KMSAN
+noinstr unsigned long return_address_noinstr(unsigned int n)
+{
+	/* Add 1 to account for call of uninlined function return_address_noinstr */
+	return return_address(n+1);
+}
+#endif
-- 
2.43.7

Re: [PATCH] kmsan: fix false warnings in return_address on s390

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Brittle backtrace logic in return_address_noinstr dependent on unpredictable compiler stack frame allocation.

Pre-existing issues:
- [High] Preexisting off-by-one error in return_address() resolution on s390x.
--

commit 6db102bd4515b51f9b3acc19ab3e650b460d0d09
Author: Aleksei Nikiforov <aleksei.nikiforov@linux.ibm.com>

kmsan: fix false warnings in return_address on s390

This commit fixes false KMSAN warnings in the s390 return_address logic
by marking a wrapper function as noinstr. It manually accounts for the
additional wrapper function's stack frame by incrementing the backtrace
depth.

> diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
> index 692c484ec1630..9a9f7f2f2c369 100644
> --- a/arch/s390/include/asm/ftrace.h
> +++ b/arch/s390/include/asm/ftrace.h
> @@ -23,7 +23,13 @@ static __always_inline unsigned long return_address(unsigned int n)
>  	} while (--n);
>  	return sf->gprs[8];

[Severity: High]
This isn't a bug introduced by this patch, but is this preexisting
off-by-one behavior correct when resolving caller IPs?

On s390x, a function's prologue saves its registers, including the return
address, into its caller's stack frame at gprs[8]. In return_address, when
n=1, the loop executes exactly once, advancing the stack frame pointer to
the immediate caller's frame. It then returns its gprs[8].

Because of the stack frame layout, this yields the instruction pointer of
the immediate caller (CALLER_ADDR0) instead of the parent caller. To
correctly return the parent caller (CALLER_ADDR1), shouldn't the loop
advance n+1 times? Lockdep and tracing paths rely on differentiating
CALLER_ADDR0 from CALLER_ADDR1, and this currently causes CALLER_ADDR1 to
evaluate to the exact same instruction pointer as CALLER_ADDR0.

>  }
> +
> +#ifdef CONFIG_KMSAN
> +unsigned long return_address_noinstr(unsigned int n);
> +#define ftrace_return_address(n) return_address_noinstr(n)
> +#else
>  #define ftrace_return_address(n) return_address(n)
> +#endif
>  
>  void ftrace_caller(void);

[ ... ]

> diff --git a/arch/s390/kernel/stacktrace.c b/arch/s390/kernel/stacktrace.c
> index 18520d3330581..db458050d8bf3 100644
> --- a/arch/s390/kernel/stacktrace.c
> +++ b/arch/s390/kernel/stacktrace.c
> @@ -146,3 +146,11 @@ void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie,
>  {
>  	arch_stack_walk_user_common(consume_entry, cookie, NULL, regs, false);
>  }
> +
> +#ifdef CONFIG_KMSAN
> +noinstr unsigned long return_address_noinstr(unsigned int n)
> +{
> +	/* Add 1 to account for call of uninlined function return_address_noinstr */
> +	return return_address(n+1);

[Severity: Medium]
Is it guaranteed that the compiler will allocate a stack frame for this
function?

This wrapper is a noinstr leaf function that only calls the
__always_inline return_address function. If the compiler optimizes it
as a standard leaf function without a stack frame, current_frame_address
inside return_address will yield the caller's stack frame instead.

If that happens, the manual +1 adjustment here will skip an extra frame
unintentionally. Does the compiler consistently allocate a stack frame
here due to sanitizer flags forcing frame pointers, or is this backtrace
logic vulnerable to unpredictable compiler heuristics?

> +}
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831152811.3817938-2-aleksei.nikiforov@linux.ibm.com?part=1