Re: [PATCH] x86: predict __access_ok() returning true

[Ingo Molnar <mingo@kernel.org>]

* Mateusz Guzik <mjguzik@gmail.com> wrote:

> On Tue, Apr 1, 2025 at 10:43 PM Ingo Molnar <mingo@kernel.org> wrote:
> >
> >
> > * Ingo Molnar <mingo@kernel.org> wrote:
> >
> > > It's also the right place to have the hint: that user addresses are
> > > valid is the common case we optimize for.
> > >
> > > Thanks,
> > >
> > >       Ingo
> > >
> > >  arch/x86/include/asm/uaccess_64.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
> > > index c52f0133425b..4c13883371aa 100644
> > > --- a/arch/x86/include/asm/uaccess_64.h
> > > +++ b/arch/x86/include/asm/uaccess_64.h
> > > @@ -54,7 +54,7 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
> > >  #endif
> > >
> > >  #define valid_user_address(x) \
> > > -     ((__force unsigned long)(x) <= runtime_const_ptr(USER_PTR_MAX))
> > > +     likely((__force unsigned long)(x) <= runtime_const_ptr(USER_PTR_MAX))
> >
> > Should we go this way, this is the safe macro variant:
> >
> >    #define valid_user_address(x) \
> >         (likely((__force unsigned long)(x) <= runtime_const_ptr(USER_PTR_MAX)))
> >
> 
> Note the are 2 tests and the other one does not get covered by *this* likely:
> valid_user_address(sum) && sum >= (__force unsigned long)ptr;
> 
> as in sum >= ptr is left be.
> 
> However, I confirmed that with your patch the issue also goes away so
> I guess it is fine.
> 
> I think it would be the safest to likely within valid_user_address()
> like in your patch, and likely on the entire expression like in mine.
> 
> That said, there will be no hard feelz if you just commit your patch
> and drop mine.

Feel free to turn it into a Co-developed-by patch:

  From: Mateusz Guzik <mjguzik@gmail.com>
  ...

  Co-developed-by: Ingo Molnar <mingo@kernel.org>
  Signed-off-by: Ingo Molnar <mingo@kernel.org>
  Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Because all I did was to transform your fix into something a bit more 
maintainable. I didn't even test it.

Thanks,

	Ingo