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

[Heiko Carstens <hca@linux.ibm.com>]

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?