Re: [RFC PATCH v1 1/2] x86/mm: use VMA lock for kernel faults on user addresses

On Mon, Aug 3, 2026 at 8:18=E2=80=AFAM Lorenzo Stoakes (ARM) <[email protected]=
g> wrote:
>
> On Sun, Aug 02, 2026 at 03:40:17PM +0800, Barry Song (Xiaomi) wrote:
> > Use the VMA lock for kernel faults on user addresses. This also
> > makes the existing code below meaningful:
> >
> >   /* Quick path to respond to signals */
> >   if (fault_signal_pending(fault, regs)) {
> >           if (!user_mode(regs))
> >                   kernelmode_fixup_or_oops(regs, error_code, address,
> >                                            SIGBUS, BUS_ADRERR,
> >                                            ARCH_DEFAULT_PKEY);
> >           return;
> >   }
>
> Hmm yeah :)
>
> Bit weird it uses user_mode(regs) and the early exit uses the just-set
> 'flags & FAULT_FLAG_USER' too.
>
> Some horrible duplication here too... the user_mod_regs() etc. code is ju=
st
> duplicated in the mmap lock path.
>
> I think you mentioned it on Suren/Dave's series but vma_start_read_unlock=
ed()
> would avoid all this and could lead to a nicely red patch.
>
> >
> > Right now, the code above is dead because !user_mode always
> > takes the mmap_lock path.
> >
> > Co-developed-by: Bo Zhang <[email protected]>
> > Signed-off-by: Bo Zhang <[email protected]>
> > Signed-off-by: Barry Song (Xiaomi) <[email protected]>
> > ---
> >  arch/x86/mm/fault.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> > index 45b99c3b1442..c22b74e0eeaf 100644
> > --- a/arch/x86/mm/fault.c
> > +++ b/arch/x86/mm/fault.c
> > @@ -1328,9 +1328,6 @@ void do_user_addr_fault(struct pt_regs *regs,
> >       }
> >  #endif
> >
> > -     if (!(flags & FAULT_FLAG_USER))
> > -             goto lock_mmap;
> > -
>
> I'm not sure why kernel faults of userland memory were excluded initially
> (Suren?)
>
> Given that it's OK to take the mmap lock here and we're necessarily in pr=
ocess
> context anyway surely it's OK to take the VMA lock?
>
> It looks fine to me but want Suren's input.

Yeah, I don't recall the specific reason it was excluded and there was
no discussion about it. Probably wanted to limit the initial series to
the most common cases and avoid any side-effects. But indeed, if it's
safe to take mmap_lock then it should be safe to take VMA lock.

>
> Also given vma_start_read_unlocked() is coming maybe that's better for a
> cleanup.
>
> And finally - Willy is working on a grand clean up of this stuff _I think=
_ so
> you probably should coordinate with him also?
>
> >       vma =3D lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > --
> > 2.39.3 (Apple Git-146)
> >
>
> --
> Cheers, Lorenzo