[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <620882a8-2b93-b709-1093-a323570f0fd2@redhat.com>
Date: Fri, 28 Apr 2023 18:00:22 +0200
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lstoakes@...il.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
Jason Gunthorpe <jgg@...pe.ca>, Jens Axboe <axboe@...nel.dk>,
Matthew Wilcox <willy@...radead.org>,
Dennis Dalessandro <dennis.dalessandro@...nelisnetworks.com>,
Leon Romanovsky <leon@...nel.org>,
Christian Benvenuti <benve@...co.com>,
Nelson Escobar <neescoba@...co.com>,
Bernard Metzler <bmt@...ich.ibm.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Bjorn Topel <bjorn@...nel.org>,
Magnus Karlsson <magnus.karlsson@...el.com>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Jonathan Lemon <jonathan.lemon@...il.com>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Christian Brauner <brauner@...nel.org>,
Richard Cochran <richardcochran@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
linux-fsdevel@...r.kernel.org, linux-perf-users@...r.kernel.org,
netdev@...r.kernel.org, bpf@...r.kernel.org,
Oleg Nesterov <oleg@...hat.com>,
Jason Gunthorpe <jgg@...dia.com>,
John Hubbard <jhubbard@...dia.com>, Jan Kara <jack@...e.cz>,
"Kirill A . Shutemov" <kirill@...temov.name>,
Pavel Begunkov <asml.silence@...il.com>,
Mika Penttila <mpenttil@...hat.com>,
David Howells <dhowells@...hat.com>,
Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v5] mm/gup: disallow GUP writing to file-backed mappings
by default
[...]
>>>
>>> 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
Powered by blists - more mailing lists