linux-kernel - Re: [PATCH v7 3/3] mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings

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] [day] [month] [year] [list]

Message-ID: <ZFGpXOWzgMifZpaJ@nvidia.com>
Date:   Tue, 2 May 2023 21:22:52 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Lorenzo Stoakes <lstoakes@...il.com>
Cc:     David Hildenbrand <david@...hat.com>,
        Peter Zijlstra <peterz@...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>,
        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>,
        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>,
        Dave Chinner <david@...morbit.com>,
        Theodore Ts'o <tytso@....edu>, Peter Xu <peterx@...hat.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Mike Rapoport <rppt@...ux.ibm.com>
Subject: Re: [PATCH v7 3/3] mm/gup: disallow FOLL_LONGTERM GUP-fast writing
 to file-backed mappings

On Tue, May 02, 2023 at 08:25:57PM +0100, Lorenzo Stoakes wrote:

> Otherwise, I'm a bit uncertain actually as to how we can get to the point
> where the folio->mapping is being manipulated. Is this why?

On RCU architectures another thread zap's the PTEs and proceeds to
teardown and free the page. Part of that is clearing folio->mapping
under the folio lock.

The IPI approach would not get there, but we can't think in terms of
IPI since we have RCU architectures.

> In any case, I will try to clarify these, I do agree the _key_ point is
> that we can rely on safely derefing the mapping, at least READ_ONCE()'d, as
> use-after-free or dereffing garbage is the fear here.

I didn't chase it down, but as long as folio->mapping is always set to
something that is RCU free'd then this would be OK.
 
> Well that was literally the question :) and I've got somewhat contradictory
> feedback. My instinct aligns with yours in that, just fallback to slow
> path, so that's what I'll do. But just wanted to confirm.

I don't know it well enough to say, it looks like folio->mapping is
almost always set to something from what I can see.. At least NULL
pointer and the anon bits set for instance.

In any case fall back to fast is always safe, I'd just go to the
trouble to check with testing that in normal process cases we don't
hit it.

> The READ_ONCE() approach is precisely how I wanted to do it in thet first
> instance, but feared feedback about duplication and wondered if it made
> much difference, but now it's clear this is ther ight way.

The duplication is bad, and maybe we need more functions to counter
it, but GUP needs to know some of the details a little more, eg a NULL
return from folio_mapping() could inidicate the anon bits being set
which should not flow down to slow.

Jason