Re: [PATCH] LoongArch: Fix calling smp_processor_id() in preemptible code

[Xi Ruoyao]

On Thu, 2026-02-26 at 17:08 +0800, Huacai Chen wrote:
> Hi, Ruoyao,
> 
> On Wed, Feb 25, 2026 at 10:15 PM Xi Ruoyao <xry111@xxxxxxxxxxx> wrote:
> > 
> > Fix the warning recently exploited by commit 4ab17e762b34 ("LoongArch:
> > BPF: Use BPF prog pack allocator"):
> > 
> >     BUG: using smp_processor_id() in preemptible [00000000] code: systemd/1
> >     caller is larch_insn_text_copy+0x40/0xf0
> > 
> > Simply changing it to raw_smp_processor_id() is not enough: if preempt
> > and CPU hotplug happens after raw_smp_processor_id() but before
> > stop_machine(), the CPU where raw_smp_processor_id() has run may be
> > offline when stop_machine() and no CPU will run copy_to_kernel_nofault()
> > in text_copy_cb().  Thus guard the larch_insn_text_copy() calls with
> > cpus_read_lock() and change stop_machine() to stop_machine_cpuslocked()
> > to prevent this.
> > 
> > Fixes: 9fbd18cf4c69 ("LoongArch: BPF: Add dynamic code modification support")
> > Signed-off-by: Xi Ruoyao <xry111@xxxxxxxxxxx>
> > ---
> > 
> > P.S. I've considered moving the locks inside larch_insn_text_copy() but
> > doing so seems not an easy hack.  In bpf_arch_text_poke() obviously the
> > memcpy() call must be guarded by text_mutex, so we have to leave the
> > acquire of text_mutex out of larch_insn_text_copy.  But in the entire
> > kernel the acquire of mutexes is always after cpus_read_lock(), so we
> > cannot put cpus_read_lock() into larch_insn_text_copy() while leaving
> > the text_mutex acquire out (or we risk a deadlock due to inconsistent
> > lock acquire order).  So I'd just submit this as-is to fix the bug and
> > leave the posssible refactor for others.
> > 
> >  arch/loongarch/kernel/inst.c | 4 ++--
> >  arch/loongarch/net/bpf_jit.c | 6 ++++++
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/loongarch/kernel/inst.c b/arch/loongarch/kernel/inst.c
> > index bf037f0c6b26..db236e2b34a7 100644
> > --- a/arch/loongarch/kernel/inst.c
> > +++ b/arch/loongarch/kernel/inst.c
> > @@ -263,14 +263,14 @@ int larch_insn_text_copy(void *dst, void *src, size_t len)
> >                 .dst = dst,
> >                 .src = src,
> >                 .len = len,
> > -               .cpu = smp_processor_id(),
> > +               .cpu = raw_smp_processor_id(),
> I think we should keep the old function here, because it can help
> others to catch new bugs if the code is refactored in future.

Hmm, IIUC using smp_processor_id will still trigger the warning as we've
only acquired the cpu hotplug lock, instead of disabling the preemption
completely.

How about adding a lockdep_assert_cpus_held() in this function with an
explanatory comment?
> 

-- 
Xi Ruoyao <xry111@xxxxxxxxxxx>