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

[Huacai Chen]

On Thu, Feb 26, 2026 at 5:32 PM Xi Ruoyao <xry111@xxxxxxxxxxx> wrote:
>
> 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?
Does this context really need preemption disabled? Why
lockdep_assert_cpus_held() can silence the warning?

Huacai

> >
>
> --
> Xi Ruoyao <xry111@xxxxxxxxxxx>
>