* [PATCH] kmsan: fix false warnings in return_address on s390
@ 2026-08-31 15:28 Aleksei Nikiforov
2026-08-31 18:48 ` sashiko-bot
2026-09-01 10:42 ` Heiko Carstens
0 siblings, 2 replies; 3+ messages in thread
From: Aleksei Nikiforov @ 2026-08-31 15:28 UTC (permalink / raw)
To: linux-s390; +Cc: 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] kmsan: fix false warnings in return_address on s390
2026-08-31 15:28 [PATCH] kmsan: fix false warnings in return_address on s390 Aleksei Nikiforov
@ 2026-08-31 18:48 ` sashiko-bot
2026-09-01 10:42 ` Heiko Carstens
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-31 18:48 UTC (permalink / raw)
To: Aleksei Nikiforov
Cc: linux-s390, Alexander Gordeev, Heiko Carstens,
Christian Borntraeger, Vasily Gorbik
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] kmsan: fix false warnings in return_address on s390
2026-08-31 15:28 [PATCH] kmsan: fix false warnings in return_address on s390 Aleksei Nikiforov
2026-08-31 18:48 ` sashiko-bot
@ 2026-09-01 10:42 ` Heiko Carstens
1 sibling, 0 replies; 3+ messages in thread
From: Heiko Carstens @ 2026-09-01 10:42 UTC (permalink / raw)
To: Aleksei Nikiforov
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev, Ilya Leoshkevich,
Juergen Christ
On Mon, Aug 31, 2026 at 05:28:12PM +0200, Aleksei Nikiforov wrote:
> 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.
...
> 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(+)
...
> +
> +#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
...
> +#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
Thinking about this again. Wouldn't READ_ONCE_NOCHECK() solve this too?
Otherwise I guess we have the same problem in the normal stack unwinder too.
Something like this:
diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 692c484ec163..4a2c46341b86 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -17,11 +17,11 @@ static __always_inline unsigned long return_address(unsigned int n)
sf = (struct stack_frame *)current_frame_address();
do {
- sf = (struct stack_frame *)sf->back_chain;
+ sf = (struct stack_frame *)READ_ONCE_NOCHECK(sf->back_chain);
if (!sf)
return 0;
} while (--n);
- return sf->gprs[8];
+ return READ_ONCE_NOCHECK(sf->gprs[8]);
}
#define ftrace_return_address(n) return_address(n)
But that might also generate an additional stack frame due to an
out-of-line call. If that doesn't help we _may_ need to come up with
an inline assembly which hides the memory access. E.g. something like
this:
static __always_inline long ___read(void *ptr)
{
long val;
asm volatile(
" lg %[val],0(%[ptr])"
: [val] "=d" (val) : [ptr] "a" (ptr));
return val;
}
and use ___read() instead of READ_ONCE_NOCHECK() within return_address().
Opinions?
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-01 10:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 15:28 [PATCH] kmsan: fix false warnings in return_address on s390 Aleksei Nikiforov
2026-08-31 18:48 ` sashiko-bot
2026-09-01 10:42 ` Heiko Carstens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.