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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4f16b1fc-6bc5-4e41-8e94-151c336fcf69@lucifer.local>
Date:   Tue, 25 Apr 2023 00:03:34 +0100
From:   Lorenzo Stoakes <lstoakes@...il.com>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     Christoph Hellwig <hch@...radead.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        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>
Subject: Re: [PATCH v2] mm/gup: disallow GUP writing to file-backed mappings
 by default

On Mon, Apr 24, 2023 at 07:53:26PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 24, 2023 at 08:18:33PM +0100, Lorenzo Stoakes wrote:
>
> > I think this patch suggestion has scope crept from 'incremental
> > improvement' to 'major rework of GUP' at this point.
>
> I don't really expect to you clean up all the callers - but we are
> trying to understand what is actually wrong here to come up with the
> right FOLL_ names and overall strategy. Leave behind a comment, for
> instance.
>

Right, but you are suggesting introducing a whole new GUP interface holding
the right locks etc. which is really scope-creeping from the original
intent.

I'm not disagreeing that we need an interface that can return things in a
state where the dirtying can be done correctly, I just don't think _this_
patch series is the place for it.

> I don't think anyone has really thought about the ptrace users too
> much till now, we were all thinking about DMA use cases, it shows we
> still have some areas that need attention.

I do like to feel that my recent glut of GUP activity, even if noisy and
frustrating, has at least helped give some insights into usage and
semantics :)

>
> > Also surely you'd want to obtain the PTL of all mappings to a file?
>
> No, just one is fine. If you do the memcpy under a single PTL that
> points at a writable copy of the page then everything is trivially
> fine because it is very similar to what the CPU itself would do, which
> is fine by definition..
>
> Jason

Except you dirty a page that is mapped elsewhere that thought everything
was cleaned and... not sure the PTLs really help you much?

Anyway I feel we're digressing into the broader discussion which needs to
be had, but not when trying to unstick the vmas series :)

I am going to put forward an opt-in variant of this change that explicitly
checks whether any VMA in the range requires dirty page tracking, if not
failing the GUP operation.

This can then form the basis of the opt-OUT variant (it'll be the same
check code right?) and help provide a basis for the additional work that
clearly needs to be done.

It will also replace the open-coded VMA check in io_uring so has utility
and justification just from that.

If we want to be more adventerous the opt-in variant could default to on
for FOLL_LONGTERM too, but that discussion can be had over on that patch
series.