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

[Mateusz Guzik]

On Tue, Apr 1, 2025 at 10:43 PM Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
>
> * Ingo Molnar <mingo@xxxxxxxxxx> 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.

> But this compiler bug sounds weird ...
>

It is very weird.

A person running a different distro with a 6.13 kernel and new gcc
confirmed they have the same bad asm, so this is not just something
screwy in my setup.

-- 
Mateusz Guzik <mjguzik gmail.com>