linux-kernel - Re: [PATCH v2 4/5] mm: make vma cache SLAB_TYPESAFE_BY

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: <CAG48ez0ZLoZno_ov=ZO3vaQnGRT6ddo6e7NmS8TpQTmy7RzmJA@mail.gmail.com>
Date: Wed, 13 Nov 2024 22:23:50 +0100
From: Jann Horn <jannh@...gle.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: "Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	Suren Baghdasaryan <surenb@...gle.com>, akpm@...ux-foundation.org, lorenzo.stoakes@...cle.com, 
	mhocko@...e.com, hannes@...xchg.org, mjguzik@...il.com, oliver.sang@...el.com, 
	mgorman@...hsingularity.net, david@...hat.com, peterx@...hat.com, 
	oleg@...hat.com, dave@...olabs.net, paulmck@...nel.org, brauner@...nel.org, 
	dhowells@...hat.com, hdanton@...a.com, hughd@...gle.com, minchan@...gle.com, 
	shakeel.butt@...ux.dev, souravpanda@...gle.com, pasha.tatashin@...een.com, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v2 4/5] mm: make vma cache SLAB_TYPESAFE_BY_RCU

On Wed, Nov 13, 2024 at 9:59 PM Matthew Wilcox <willy@...radead.org> wrote:
> On Wed, Nov 13, 2024 at 05:44:00PM +0100, Jann Horn wrote:
> > Something like NULL or (void*)1 is fine with me but please don't do
> > pointer-to-itself - we shouldn't unnecessarily store a pointer to an
> > object of one type in a pointer field of an incompatible type, that
> > increases the risk of creating type confusion issues (both in the
> > memory corruption sense and in the Spectre sense). I know MM already
> > has several places where similar stuff can happen (in particular
> > page->mapping), but here it seems like unnecessary risk to me.
>
> Hm?  I don't think page->mapping can ever point at page.  As far as I
> know, we have four cases, discriminated by the bottom two bits:
>
> 0 - NULL or address_space
> 1 - anon_vma
> 2 - movable_ops
> 3 - ksm_stable_node

Oh, I didn't even know about cases 2 and 3.

Ah, yes, I didn't mean it can point at itself, I meant the pattern of
having a pointer declared as "points to type A" ("struct address_space
*mapping") while it actually points at other types sometimes.

> In fact, we're almost done eliminating page->mapping.  Just a few
> filesystems and device drivers left to go.

Ah, you mean by replacing it with folio->mapping as in
https://lore.kernel.org/all/20241025190822.1319162-4-willy@infradead.org/
?

> Would it be halpful if we did:
>
> -       struct address_space *mapping;
> +       union {
> +               struct address_space *mapping;
> +               unsigned long raw_mapping;
> +       };
>
> and had non-filesystems use raw_mapping and do the masking?

While I think that would look a tiny bit tidier, I don't think it
would make a significant difference for page->mapping or
folio->mapping. In the context of the VMA field, the question is about
whether we make it possible for the same memory location to contain
values of different types, which I think increases the risk of
programmer mistakes or speculative confusions; while in the context of
the page->mapping field, the same memory location can contain values
of different types either way. So while aesthetically I think having a
union for the mapping field would look a little nicer, I don't
actually see much benefit in making such a change.