Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3f97c798-3598-1729-1981-ab8acb7b5663@redhat.com>
Date:   Fri, 28 Apr 2023 17:41:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     Lorenzo Stoakes <lstoakes@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Jens Axboe <axboe@kernel.dk>,
        Matthew Wilcox <willy@infradead.org>,
        Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>,
        Leon Romanovsky <leon@kernel.org>,
        Christian Benvenuti <benve@cisco.com>,
        Nelson Escobar <neescoba@cisco.com>,
        Bernard Metzler <bmt@zurich.ibm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Ian Rogers <irogers@google.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Bjorn Topel <bjorn@kernel.org>,
        Magnus Karlsson <magnus.karlsson@intel.com>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Jonathan Lemon <jonathan.lemon@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Christian Brauner <brauner@kernel.org>,
        Richard Cochran <richardcochran@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        John Fastabend <john.fastabend@gmail.com>,
        linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        netdev@vger.kernel.org, bpf@vger.kernel.org,
        Oleg Nesterov <oleg@redhat.com>,
        John Hubbard <jhubbard@nvidia.com>, Jan Kara <jack@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Pavel Begunkov <asml.silence@gmail.com>,
        Mika Penttila <mpenttil@redhat.com>,
        David Howells <dhowells@redhat.com>,
        Christoph Hellwig <hch@lst.de>
References: <6b73e692c2929dc4613af711bdf92e2ec1956a66.1682638385.git.lstoakes@gmail.com>
 <afcc124e-7a9b-879c-dfdf-200426b84e24@redhat.com>
 <ZEvZtIb2EDb/WudP@nvidia.com>
 <094d2074-5b69-5d61-07f7-9f962014fa68@redhat.com>
 <ZEvl717EANEu8113@nvidia.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
Subject: Re: [PATCH v5] mm/gup: disallow GUP writing to file-backed mappings
 by default
In-Reply-To: <ZEvl717EANEu8113@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 28.04.23 17:27, Jason Gunthorpe wrote:
> On Fri, Apr 28, 2023 at 05:08:27PM +0200, David Hildenbrand wrote:
> 
>>> I think this is broken today and we should block it. We know from
>>> experiments with RDMA that doing exactly this triggers kernel oop's.
>>
>> I never saw similar reports in the wild (especially targeted at RHEL), so is
>> this still a current issue that has not been mitigated? Or is it just so
>> hard to actually trigger?
> 
> People send RDMA related bug reports to us, and we tell them not to do
> this stuff :)
> 
>>> I'm skeptical that anyone can actually do this combination of things
>>> successfully without getting kernel crashes or file data corruption -
>>> ie there is no real user to break.
>>
>> I am pretty sure that there are such VM users, because on the libvirt level
>> it's completely unclear which features trigger what behavior :/
> 
> IDK, why on earth would anyone want to do this? Using VFIO forces all
> the memory to become resident so what was the point of making it file
> backed in the first place?

As I said, copy-and paste, incremental changes to domain XMLs. I've seen 
some crazy domain XMLs in bug reports.

> 
> I'm skeptical there are real users even if it now requires special
> steps to be crashy/corrupty.

In any case, I think we should document the possible implications of 
this patch. I gave one use case that could be broken.

> 
>>>> Sure, we could warn, or convert individual users using a flag (io_uring).
>>>> But maybe we should invest more energy on a fix?
>>>
>>> It has been years now, I think we need to admit a fix is still years
>>> away. Blocking the security problem may even motivate more people to
>>> work on a fix.
>>
>> Maybe we should make this a topic this year at LSF/MM (again?). At least we
>> learned a lot about GUP, what might work, what might not work, and got a
>> depper understanding (+ motivation to fix? :) ) the issue at hand.
> 
> We keep having the topic.. This is the old argument that the FS people
> say the MM isn't following its inode and dirty lifetime rules and the
> MM people say the FS isn't following its refcounting rules <shrug>

:/ so we have to discuss it ... again I guess.

> 
>>> Security is the primary case where we have historically closed uAPI
>>> items.
>>
>> As this patch
>>
>> 1) Does not tackle GUP-fast
>> 2) Does not take care of !FOLL_LONGTERM
>>
>> I am not convinced by the security argument in regard to this patch.
> 
> It is incremental and a temperature check to see what kind of real
> users exist. We have no idea right now, just speculation.

Right, but again, if we start talking about security it's a different 
thing IMHO.

>> Everything else sounds like band-aids to me, is insufficient, and might
>> cause more harm than actually help IMHO. Especially the gup-fast case is
>> extremely easy to work-around in malicious user space.
> 
> It is true this patch should probably block gup_fast when using
> FOLL_LONGTERM as well, just like we used to do for the DAX check.

Then we'd at least fix the security issue for all FOLL_LONGTERM completely.

-- 
Thanks,

David / dhildenb