Re: [syzbot] [kernel?] upstream test error: KMSAN: uninit-value in irqentry_exit_to_kernel_mode_preempt

[Alexander Potapenko]

On Wed, May 13, 2026 at 2:36 AM Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
>
> Alexander!


>
> Thanks for the explanation.
>
> TBH, I'm failing to see how this is not inSANe, but I might be missing
> something obvious.
>
> What really bothers me is the lack of distinction between arguments
> passed by value vs. arguments passed by reference. For me there is a
> clear distinction.
>
> Any argument passed by reference, i.e. pointer, obviously requires to be
> validated at the actual usage site, which might be several levels deep
> into the call chain. That obviously does not include by value arguments
> which are passed on the stack due to exceeding the register passing
> constraints,
>
> But for arguments passed by value, I fail to see the justification.
>
> The compiler can validate at the call site that the value arguments are
> properly initialized both at compile and at runtime:
>
>     1) If the argument value is a constant it is initialized
>
>     2) If the argument value is read from memory at the call site, then
>        the function which reads has to validate that the memory is
>        initialized before reading it.
>
>     3) If the argument value was returned from a previous function call
>        then that invoked function has to make sure that the returned
>        value is properly initialized.
>
>     4) If the argument is a local variable which is not initialized
>        that's catched at compile time with and without KMSAN. If people
>        ignore that warning, then they will ignore the resulting KMSAN
>        splat too.
>
>     5) If the argument is only propagated to the next call, then #1 - #4
>        apply to the caller, i.e. all of that propagates through the
>        call chain.
>
>     6) If the argument is handed in from the previous caller and
>        modified by the function which calls the next one, then still #1
>        - #4 apply.
>
> IOW, the producer of a value argument has to ensure that the value
> argument is properly initialized.
>
> This whole 'is initialized' propagation for by value arguments is
> overengineered voodoo in my opinion. Why?

Sorry, I left out a big chunk of how KMSAN works.
In fact, the above cases are covered by
`-fsanitize-memory-param-retval`, which eagerly checks parameters and
return values having the `noundef` attribute.
For such cases, the compiler will not propagate the shadow into TLS.

Yet we still have to maintain TLS parameter passing for types that
lack the `noundef` attribute (e.g. structs with internal padding or
vector types) and for variadic function arguments.

>
>   If the compiler can't ensure at the producer site that something is
>   properly initialized and then can't ensure that the intermediates
>   handing the value argument down the callchain are doing the right
>   thing, then KMSAN won't help either as it is then by definition
>   equally untrustworthy as the untrustworthy compiler which emitted the
>   KMSAN self-validation along with the broken code.

Agreed.

>   So it just adds overhead for nothing and makes everything look "sane"
>   while in fact it provides no value and creates exactly the problems we
>   are debating right now.
>
> No?

I hope the above addresses this concern to some extent.
While eager checks simplify the instrumentation code, they (as
mentioned) do not completely remove the need for TLS parameter
passing.
Long story short, it's unsafe to assume at the beginning of an
instrumented function that all its by-value arguments are initialized.
If that were true, we would indeed have no problems with
non-instrumented functions calling instrumented ones.


> >
> > Now, this works fine under the assumption that all code in the kernel
> > is instrumented with KMSAN.
> > However this is not true.
>
> We know that since KMSAN got integrated and you pointed yourself to that
> discussion which we had back then about handling the @regs pointer
> argument.
>
> There was a brief mentioning of the same problem about non-pointer
> arguments, but that dried out because the test runs did not barf anymore
> after the @regs unpoisoning got fixed.
>
> There were discussions about utilizing instrumentation_begin()/end() to
> tell the compiler that this instrumentation_begin() lifts the 'do not
> sanitize directive' and it could rightfully inject instrumentation code
> there including calls, but obviously nothing happened.

Right. At that point, we thought __no_kmsan_checks should be enough to
keep the complexity manageable.
The more I think about it now, the more I am convinced it shouldn't be
hard to implement intrinsics that
allow instrumentation between
instrumentation_begin()/instrumentation_end() and treat all values
passed into that region as initialized.
This should fix problems with noinstr functions calling instrumented functions.

> What really worries me more than this particular problem at hand is that
> we have tons of places which invoke instrumented functions from
> non-instrumented code with arguments originating from the
> non-instrumented code. Here is an example:
>
> DEFINE_IDTENTRY_ERRORCODE(exc_invalid_tss)
> {
>         do_error_trap(regs, error_code, "invalid TSS", X86_TRAP_TS, SIGSEGV,
>                       0, NULL);
> }
>
> #define DEFINE_IDTENTRY_ERRORCODE(func)                                 \
> static __always_inline void __##func(struct pt_regs *regs,              \
>                                      unsigned long error_code);         \
>                                                                         \
> __visible noinstr void func(struct pt_regs *regs,                       \
>                             unsigned long error_code)                   \
> {                                                                       \
>         irqentry_state_t state = irqentry_enter(regs);                  \
>                                                                         \
>         instrumentation_begin();                                        \
>         __##func (regs, error_code);                                    \
>         instrumentation_end();                                          \
>         irqentry_exit(regs, state);                                     \
> }                                                                       \
>                                                                         \
> static __always_inline void __##func(struct pt_regs *regs,              \
>                                      unsigned long error_code)
>
> @error_code comes all the way from the ASM entry code.
>
> The function which gets inlined within the instrumentation_begin()/end()
> region in the non-instrumented exc_invalid_tss() is:
>
> static __always_inline void __exc_invalid_tss(struct pt_regs *regs, unsigned long error_code)
> {
>         do_error_trap(regs, error_code, "invalid TSS", X86_TRAP_TS, SIGSEGV, 0, NULL);
> }
>
> Anything else than @regs (already unpoisoned) and @error_code is compile
> time constant.
>
> do_error_trap() is in the same compilation unit, static and
> instrumentable:
>
> static void do_error_trap(struct pt_regs *regs, long error_code, char *str,
>         unsigned long trapnr, int signr, int sicode, void __user *addr)
> {
>         RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
>
>         if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
>                         NOTIFY_STOP) {
>                 cond_local_irq_enable(regs);
>                 do_trap(trapnr, signr, str, regs, error_code, sicode, addr);
>                 cond_local_irq_disable(regs);
>         }
> }
>
> That should suffer from the very same problem vs. _all_ arguments
> except @regs, right?
>
> But no, it does not. The same compilers which emit imaginary initialized
> checks for something they out of lined themself despite knowing that the
> only calling context is not instrumentable, ignore the very same for
> do_error_trap().
>
> According to the disassembly do_error_trap() just goes and invokes
> notify_die() without any checks and notify_die() takes the same
> (reordered) arguments uninspected and stores them into a on stack data
> struct which is properly poisened first and then unpoisoned for each
> member stored.

Let's look at the compilation result with `-S -emit-llvm` - it is
generally easier to comprehend and gives better understanding of
what's going on with the instrumentation:

; Function Attrs: fn_ret_thunk_extern noredzone nounwind
null_pointer_is_valid sanitize_memory
define internal fastcc void @do_error_trap(ptr noundef %regs, i64
noundef %error_code, ptr noundef %str, i64 noundef range(i64 0, 13)
%trapnr, i32 noundef range(i32 4, 12) %signr, i32 noundef range(i32 0,
3) %sicode, ptr noundef %addr) unnamed_addr #9 align 16 !dbg !10395 {
entry:
  %0 = call ptr @__msan_get_context_state() #19, !dbg !11994
  %retval_shadow = getelementptr i8, ptr %0, i64 800, !dbg !11994
  %param_origin = getelementptr i8, ptr %0, i64 3208, !dbg !11994
  %retval_origin = getelementptr i8, ptr %0, i64 4008, !dbg !11994
    #dbg_value(ptr %regs, !10394, !DIExpression(), !11995)
    #dbg_value(i64 %error_code, !10399, !DIExpression(), !11995)
    #dbg_value(ptr %str, !10400, !DIExpression(), !11995)
    #dbg_value(i64 %trapnr, !10401, !DIExpression(), !11995)
    #dbg_value(i32 %signr, !10402, !DIExpression(), !11995)
    #dbg_value(i32 %sicode, !10403, !DIExpression(), !11995)
    #dbg_value(ptr %addr, !10404, !DIExpression(), !11995)
  call void @__sanitizer_cov_trace_pc() #20, !dbg !11994
  %conv = trunc nuw nsw i64 %trapnr to i32, !dbg !11996
  store i32 0, ptr %retval_shadow, align 8, !dbg !11997
  %call = call i32 @notify_die(i32 noundef 8, ptr noundef %str, ptr
noundef %regs, i64 noundef %error_code, i32 noundef %conv, i32 noundef
%signr) #21, !dbg !11997
...

There are no checks for `@notify_die` arguments because
`@do_error_trap` receives them as `noundef`.
If eager checks are enabled, KMSAN pass knows that these arguments
were already checked previously (see
https://www.llvm.org/doxygen/MemorySanitizer_8cpp_source.html#l02149),
so it omits the checks.

`@do_error_trap` is not instrumented, but even if it were, the check
for `%error_code` would have been pushed to its caller because of the
`noundef` attribute.

; Function Attrs: disable_sanitizer_instrumentation
fn_ret_thunk_extern noinline noprofile noredzone nosanitize_coverage
nounwind null_pointer_is_valid
define dso_local void @exc_invalid_tss(ptr noundef %regs, i64 noundef
%error_code) local_unnamed_addr #4 section ".noinstr.text" align 16
!dbg !10467 {
entry:
    #dbg_value(ptr %regs, !10469, !DIExpression(), !10472)
    #dbg_value(i64 %error_code, !10470, !DIExpression(), !10472)
  %call = call i8 @irqentry_enter(ptr noundef %regs) #21, !dbg !10473
    #dbg_value(i8 %call, !10471, !DIExpression(), !10472)
  call void asm sideeffect "801: nop\0A\09.pushsection
.discard.annotate_insn, \22M\22, @progbits, 8; .long 801b - ., 3;
.popsection", "i,~{dirflag},~{fpsr},~{flags}"(i32 801) #19, !dbg
!10474, !srcloc !10477
    #dbg_value(ptr %regs, !10478, !DIExpression(), !10482)
    #dbg_value(i64 %error_code, !10481, !DIExpression(), !10482)
  call fastcc void @do_error_trap(ptr noundef %regs, i64 noundef
%error_code, ptr noundef nonnull @.str.15, i64 noundef 10, i32 noundef
11, i32 noundef 0, ptr noundef null) #22, !dbg !10485
  call void asm sideeffect "802: nop\0A\09.pushsection
.discard.annotate_insn, \22M\22, @progbits, 8; .long 802b - ., 4;
.popsection", "i,~{dirflag},~{fpsr},~{flags}"(i32 802) #19, !dbg
!10486, !srcloc !10489
  call void @irqentry_exit(ptr noundef %regs, i8 %call) #21, !dbg !10490
  ret void, !dbg !10473
}

So, if a `noundef` value is passed along a chain of function calls, it
is checked only once, at the point where we don't yet know that it is
`noundef`.
As mentioned, for `noundef` arguments, no shadow is stored in the TLS.
Thus, even if non-instrumented functions are along the way, their
callees still assume that the `noundef` argument is initialized (which
may introduce false negatives, but these are inevitable when
non-instrumented code is present).
To achieve the same behavior for the function argument with TLS shadow
propagation, we'll indeed need to enable instrumentation within the
instrumentation_begin()/instrumentation_end() region.

I think doing so will resolve the issues with instrumented functions
being called from non-instrumented functions in the same KMSAN
context, and will not require the call_instr() magic you are
suggesting.
So let me finally take a stab at it.

Note that the issue with incorrect context tracking in [soft]irqs is
orthogonal to this and will need to be addressed separately.