Re: [PATCH v3] fs: Replace user_access_{begin/end} by scoped user access

[David Laight]

On Mon, 16 Mar 2026 10:12:23 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Mon, 16 Mar 2026 at 01:53, Christophe Leroy (CS GROUP)
> <chleroy@xxxxxxxxxx> wrote:
> >
> > -       if (!user_write_access_begin(dirent,
> > -                       (unsigned long)(dirent->d_name + namlen + 1) -
> > -                               (unsigned long)dirent))
> > -               goto efault;  
> 
> This was already pretty unreadable (my bad), but..
> 
> > +       scoped_user_write_access_size(dirent, (unsigned long)(dirent->d_name + namlen + 1) -
> > +                                             (unsigned long)dirent, efault) {  
> 
> .. in my opinion, this is even worse. It's a 90+ character long line
> (or something), *and* it continues on the next line.
> 
> Yes, yes, the old code was disgusting too, but when changing it for
> something that is supposed to be easier to read, please let's make it
> *really* easier to read.
> 
> (And yes, there's another case of this same pattern a bit later).
> 
> Adding a helper inline function like dirent_size() might do it. And I
> think it might as well be cleaned up while at it, and make it be
> something like
> 
>     static inline size_t dirent_size(int namelen)
>     {
>         return offsetof(struct linux_dirent, d_name) + namelen + 1;
>     }

That definition would fit one one line (possibly as a continuation line).

> [ Making things doubly sad, the size shouldn't even be *used* in sane
> situations, because the user access validity is often checked by only
> checking the beginning,
> 
>   The x86-64 __access_ok() does actually do that optimization, but
> only if we can statically see that the thing is smaller than one page,
> which in this case it can't, because while namelen is range checked,
> it is allowed to be up to PATH_MAX in size, so even if the compiler
> does do the full value range analysis, we'd need to relax that
> __access_ok() check a bit more ]

Except is doesn't matter for x86-64 because this is the 'masked' case
where the accesses are required to be 'reasonably sequential'.

Although requiring a guard page on all architectures would make life
simpler overall.
I'm not even sure that some of the reasons that x86-64 has to have
a guard page (to stop speculative execution and system call return
to non-canonical addresses) don't have potential counterparts on x86-32
and other architectures. 

For x86-32 I believe that historically the user stack was right at the
top of the user address space (or the constant wouldn't be STACK_TOP).
So forcing a guard page would realign the stack.
But I've not got an x86-32 system setup (I've got plenty of hardware)
so see what actually happens or test any hacking.
I'm also not sure where the vdso ends up - I'd have thought it might
be right at the top?

	David

> 
> Hmm? Can you do at least that dirent_size() kind of cleanup?
> 
>               Linus