Re: [PATCH v5] mm/gup: disallow GUP writing to file-backed mappings by default

From: David Hildenbrand
Date: Fri Apr 28 2023 - 12:01:13 EST


[...]


Personally I come at this from the 'I just want my vmas patch series' unblocked
perspective :) and feel there's a functional aspect here too.

I know, it always gets messy when touching such sensible topics :P

I feel that several people owe me drinks at LSF/MM :P

To cut a long story short to your other points, I'm _really_ leaning
towards an opt-in variant of this change that we just hand to io_uring to
make everything simple with minimum risk (if Jens was also open to this
idea, it'd simply be deleting the open coded vma checks there and adding
FOLL_SAFE_FILE_WRITE).

That way we can save the delightful back and forth for another time while
adding a useful feature and documenting the issue.

Just for the records: I'm not opposed to disabling it system-wide, especially once this is an actual security issue and can bring down the machine easily (thanks to Jason for raising the security aspect). I just wanted to raise awareness that there might be users affected ...

Sure, we could glue this to some system knob like Jason said, if we want to play safe.


Altneratively I could try to adapt this to also do the GUP-fast check,
hoping that no FOLL_FAST_ONLY users would get nixed (I'd have to check who
uses that). The others should just get degraded to a standard GUP right?

Yes. When you need the VMA to make a decision, fallback to standard GUP.

The only problematic part is something like get_user_pages_fast_only(), that would observe a change. But KVM never passes FOLL_LONGTERM, so at least in that context the change should be fine I guess.

The performance concern is the most problematic thing (how to identify shmem pages).


I feel these various series have really helped beat out some details about
GUP, so as to your point on another thread (trying to reduce noise here
:P), I think discussion at LSF/MM is also a sensible idea, also you know,
if beers were bought too it could all work out nicely :]

The issue is, that GUP is so complicated, that each and every MM developer familiar with GUP has something to add :P

What stood out to me is that we disallow something for ordinary GUP but disallow it for GUP-fast, which looks very odd.

So sorry again for jumping in late ...

--
Thanks,

David / dhildenb