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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ