Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Jan 07, 2021 at 04:45:33PM -0500, Andrea Arcangeli wrote:
> On Thu, Jan 07, 2021 at 04:25:25PM -0400, Jason Gunthorpe wrote:
> > On Thu, Jan 07, 2021 at 03:04:00PM -0500, Andrea Arcangeli wrote:
> > 
> > > vmsplice syscall API is insecure allowing long term GUP PINs without
> > > privilege.
> > 
> > Lots of places are relying on pin_user_pages long term pins of memory,
> > and cannot be converted to notifiers.
> > 
> > I don't think it is reasonable to just declare that insecure and
> > requires privileges, it is a huge ABI break.
> 
> Where's that ABI? Are there specs or a code example in kernel besides
> vmsplice itself?

If I understand you right, you are trying to say that the 193
pin_user_pages() callers cannot exist as unpriv any more?

The majority cannot be converted to notifiers because they are DMA
based. Every one of those is an ABI for something, and does not expect
extra privilege to function. It would be a major breaking change to
have pin_user_pages require some cap.

> The whole zygote issue wouldn't even register if the child had the
> exact same credentials of the parent. Problem is the child dropped
> privileges and went with a luser id, that clearly cannot ptrace the
> parent, and so if long term unprivileged GUP pins are gone from the
> equation, what remains that the child can do is purely theoretical
> even before commit 17839856fd588f4ab6b789f482ed3ffd7c403e1f.

Sorry, I'm not sure I've found a good explanation how ptrace and GUP
are interacting to become a security problem.

17839 makes sense to me, and read-only GUP has been avoided by places
like RDMA and others for a very long time because of these issues,
adding the same idea to the core code looks OK.

The semantics we discussed during the COW on fork thread for pin user
pages were, more or less, that once pinned a page should not be
silently removed from the mm it is currently in by COW or otherwise in
the kernel.

So maybe ptrace should not be COW'ing pinned pages at all, as that is
exactly the same kind of silent corruption fork was causing.

Jason