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

David Hildenbrand
2023-04-28T16:00:36Z

[...]

>>>
>>> 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