Re: [PATCH v2 01/17] mm/gup: Fixup p*_access_permitted()

[Linus Torvalds]

On Thu, Dec 14, 2017 at 11:51 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> So we actually need the pte_access_permitted() stuff if we want to
> ensure we're not stepping on !PAGE_USER things.

We really don't. Not in that complex and broken format, and not for every level.

Also, while I think we *should* check the PAGE_USER bit when walking
the page tables, like we used to, we should

 (a) do it much more simply, not with that broken interface that takes
insane and pointless flags

 (b) not tie it together with this issue at all, since the PAGE_USER
thing really is largely immaterial.

The fact is, if we have non-user mappings in the user part of the
address space, we _need_ to teach access_ok() about them, because
fundamentally any "get_user()/put_user()" will happily ignore the lack
of PAGE_USER (since those happen from kernel space).

So I'd like to check PAGE_USER in GUP simply because it's a simple
sanity check, not because it is important.

And that whole "p??_access_permitted() checks against the current
PKRU" is just incredible shit. It's currently broken, exactly because
"current PKRU" isn't even well-defined when you do it across different
threads, much less different address spaces.

This is why I'm 100% convinced that the current
"p??_access_permitted()" is just pure and utter garbage. And it's
garbage at a _fundamental_ level, not because of some small
implementation detail.

            Linus