> x86: Use RCU in all users of __module_address(). > > __module_address() can be invoked within a RCU section, there is no > requirement to have preemption disabled. > > Replace the preempt_disable() section around __module_address() with > RCU. > > Cc: H. Peter Anvin <[email protected]> > Cc: Borislav Petkov <[email protected]> > Cc: Dave Hansen <[email protected]> > Cc: Ingo Molnar <[email protected]> > Cc: Josh Poimboeuf <[email protected]> > Cc: Peter Zijlstra <[email protected]> > Cc: Thomas Gleixner <[email protected]> > Cc: [email protected] > Acked-by: Peter Zijlstra (Intel) <[email protected]> > Signed-off-by: Sebastian Andrzej Siewior <[email protected]> > --- > arch/x86/kernel/callthunks.c | 3 +-- > arch/x86/kernel/unwind_orc.c | 4 +--- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > index f17d166078823..276b5368ff6b0 100644 > --- a/arch/x86/kernel/callthunks.c > +++ b/arch/x86/kernel/callthunks.c > @@ -98,11 +98,10 @@ static inline bool within_module_coretext(void *addr) > #ifdef CONFIG_MODULES > struct module *mod; > > - preempt_disable(); > + guard(rcu)(); > mod = __module_address((unsigned long)addr); > if (mod && within_module_core((unsigned long)addr, mod)) > ret = true; > - preempt_enable(); > #endif > return ret; > } > diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c > index d4705a348a804..977ee75e047c8 100644 > --- a/arch/x86/kernel/unwind_orc.c > +++ b/arch/x86/kernel/unwind_orc.c > @@ -476,7 +476,7 @@ bool unwind_next_frame(struct unwind_state *state) > return false; > > /* Don't let modules unload while we're reading their ORC data. */ > - preempt_disable(); > + guard(rcu)(); > > /* End-of-stack check for user tasks: */ > if (state->regs && user_mode(state->regs)) > @@ -669,14 +669,12 @@ bool unwind_next_frame(struct unwind_state *state) > goto err; > } > > - preempt_enable(); > return true;
Hi, There is a regression report on a distribution forum which involves an out of tree module on a patched kernel (yes, I know) calling stack_trace_save() in task context, which arrives here and apparently calls the various deref_stack_xxx() functions with preemption enabled, which in turn call stack_access_ok() leading to a BUG: Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183 Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60 Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P OE 6.16.12-hardened1-1-hardened #1 PREEMPT(full) 6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016 Nov 02 21:44:30 ArchBasement kernel: Call Trace: Nov 02 21:44:30 ArchBasement kernel: <TASK> Nov 02 21:44:30 ArchBasement kernel: dump_stack_lvl+0x5d/0x80 Nov 02 21:44:30 ArchBasement kernel: check_preemption_disabled+0xe5/0xf0 Nov 02 21:44:30 ArchBasement kernel: in_entry_stack+0x11/0x60 Nov 02 21:44:30 ArchBasement kernel: get_stack_info+0x2c/0x80 Nov 02 21:44:30 ArchBasement kernel: stack_access_ok+0x51/0xa0 Nov 02 21:44:30 ArchBasement kernel: unwind_next_frame+0x1cb/0x7b0 Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2] Nov 02 21:44:30 ArchBasement kernel: ? __pfx_stack_trace_consume_entry+0x10/0x10 Nov 02 21:44:30 ArchBasement kernel: arch_stack_walk+0xa6/0x110 Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2] Nov 02 21:44:30 ArchBasement kernel: stack_trace_save+0x4d/0x70 Is this nvidia doing something wrong, or a problem with this commit? The removed code suggests that preemption is allowed here, and as far as I see, this call trace is still possible on vanilla 6.18. Perhaps preempt_disable() needs to be restored around this code? Regards, Michal
