Messages in this thread Patch in this message | | | From | Thomas Gleixner <> | Subject | Re: [PATCH v3 28/46] kmsan: entry: handle register passing from uninstrumented code | Date | Thu, 12 May 2022 18:48:38 +0200 |
| |
On Thu, May 12 2022 at 18:17, Thomas Gleixner wrote: > On Thu, May 12 2022 at 14:24, Alexander Potapenko wrote: >> We could try to figure out the places in idtentry code where normal >> kmsan_unpoison_memory() can be called in IRQ context, but as far as I >> can see it will depend on the type of the entry point. > > NMI is covered as it increments before it invokes the unpoison(). > > Let me figure out why we increment the preempt count late for > interrupts. IIRC it's for symmetry reasons related to softirq processing > on return, but let me double check.
It's even documented:
https://www.kernel.org/doc/html/latest/core-api/entry.html#interrupts-and-regular-exceptions
But who reads documentation? :)
So, I think the simplest and least intrusive solution is to have special purpose unpoison functions. See the patch below for illustration.
The reasons why I used specific ones:
1) User entry
Whether that's a syscall or interrupt/exception does not matter. It's always on the task stack and your machinery cannot be running at that point because it came from user space.
2) Interrupt/exception/NMI entry kernel
Those can nest into an already active context, so you really want to unpoison @regs.
Also while regular interrupts cannot nest because of interrupts staying disabled, exceptions triggered in the interrupt handler and NMIs can nest.
-> device interrupt() irqentry_enter(regs)
-> NMI() irqentry_nmi_enter(regs)
-> fault() irqentry_enter(regs)
--> debug_exception() irqentry_nmi_enter(regs)
Soft interrupt processing on return from interrupt makes it more interesting:
interrupt() handler() do_softirq() local_irq_enable() interrupt() NMI ....
And everytime you get a new @regs pointer to deal with.
Wonderful, isn't it?
Thanks,
tglx
--- --- a/kernel/entry/common.c +++ b/kernel/entry/common.c @@ -24,6 +24,7 @@ static __always_inline void __enter_from user_exit_irqoff(); instrumentation_begin(); + unpoison_user(regs); trace_hardirqs_off_finish(); instrumentation_end(); } @@ -352,6 +353,7 @@ noinstr irqentry_state_t irqentry_enter( lockdep_hardirqs_off(CALLER_ADDR0); rcu_irq_enter(); instrumentation_begin(); + unpoison_irq(regs); trace_hardirqs_off_finish(); instrumentation_end(); @@ -367,6 +369,7 @@ noinstr irqentry_state_t irqentry_enter( */ lockdep_hardirqs_off(CALLER_ADDR0); instrumentation_begin(); + unpoison_irq(regs); rcu_irq_enter_check_tick(); trace_hardirqs_off_finish(); instrumentation_end(); @@ -452,6 +455,7 @@ irqentry_state_t noinstr irqentry_nmi_en rcu_nmi_enter(); instrumentation_begin(); + unpoison_irq(regs); trace_hardirqs_off_finish(); ftrace_nmi_enter(); instrumentation_end();
| |