Re: [git pull] uaccess-related bits of vfs.git

Al Viro
2017-05-13T17:00:59Z

On Sat, May 13, 2017 at 09:15:07AM -0700, Linus Torvalds wrote:
> > IOW, we have
> > * most of users in arch/* (heavily dominated by signal-related code,
> > both loads and stores). Those need careful massage; maybe unsafe-based
> > solution, maybe something else, but it's obviously per-architecture work
> > and these paths are sensitive.
>
> Why are they sensitive?

Because there we do have tons of back-to-back __get_user() (on sigreturn())
or __put_user() (on signal setup).

> Why not just do this:
>
> git grep -l '\<__\(\(get\)\|\(put\)\)_user(' -- arch/x86
> :^arch/x86/include/asm/uaccess.h
> | xargs sed -i 's/__\(\(\(get\)\|\(put\)\)_user(\)/\1/g'
>
> which converts all the x86 uses in one go.

x86 actually tends to use get_user_ex/put_user_ex instead.

> Anybody who *relies* on not checking the address_limit is so broken as
> to be not even funny.

Except for open-coded probe_kernel_read() somewhere in arch/*; I have not
read through those 700+ callers, so I don't know if there's any such.
Sure, those won't be performance-sensitive.

> And anything that is so performance-sensitive
> that anybody can even measure the effect of the above we can convert
> later.

> Sure, do it in pieces (eg each architecture separately, then
> "drivers", then "networking", then whatever, until all done), just so
> that *if* something actually depends on semantics (and that really
> shouldn't be the case), we have at least some information from a
> bisect.
>
> But I don't see the excuse for not just doing it. If nobody notices,
> it's an obvious improvement. And if somebody *does* notice, we know
> how to do it properly with unsafe_xyz_user(), because "__xyz_user()"
> most definitely isn't it.

I think we ought to actually look through those places - there are few
enough of them (outside of arch/, that is) and stac/clac overhead is
not the only problem they tend to have.