linux-kernel - Re: [PATCH v1] kernel/fork: only call untrack_pfn

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: <9a9103db-95ca-47d4-921e-42b8368ffcaf@lucifer.local>
Date: Thu, 24 Apr 2025 13:49:02 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
        Rik van Riel <riel@...riel.com>, "H. Peter Anvin" <hpa@...or.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v1] kernel/fork: only call untrack_pfn_clear() on VMAs
 duplicated for fork()

On Thu, Apr 24, 2025 at 02:33:54PM +0200, David Hildenbrand wrote:
> > >
> > > ... and I think we still have space in vm_area_struct without increasing it
> > > beyond 192 bytes.
> >
> > Hm, so you're thinking of a general field in the VMA? I thought this would
> > belong to the PAT object somehow?
>
> It's glued to a VMA. The only alternative to using a VMA field would be looking it
> up for a VMA, storing it in an xarray etc ... ends up complicating stuff when there
> is no need to right now.

Yeah no, that'd be crazy.

I guess we are stuck with this so need to think of a sensible way forward.

>
> >
> > Though getting rid of VM_PAT would be fantastic...
> >
> > I wonder if a _general_ VMA ref count would be a bit much just for this
> > case.
>
> I don't think it would be helpful for this case. It's much more similar to the anon
> VMA name (that also has its own kref)

Ahh I see, adding a field like this, that makes a lot more sense, obviously
could put behind appropriate CONFIG_ flag.

Given cacheline alignment probably have a bit of wiggle room there to avoid
increasing VMA size also.

>
> >
> > But maybe I misunderstand your approach :) Happy to obviously look and if
> > not like some crazy thing just for PAT (you can understand why I would not
> > like this) will be supportive :>)
>
> This is something quick (well, longer than I wish it would take) that seems to
> work. There are smaller pat-internal cleanups to be had on top of this, and
> the new functions shall be documented.
>
>
> Observe how:
> * We remove VM_PAT and that weird VM flags manipulation + "locked" flag
> * We remove any traces of the nasty tracking handling from mremap+fork code
> * Just like anon_vma_name, it hooks into vm_area_dup()/vm_area_free().
> * We remove the page table lookup via get_pat_info()->... completely
> * We remove the VMA parameter from PAT code completely
> * We reduce the track/untrack/sanitize interface to 3 functions

Yeah this is all lovely!

OK this should hopefully be workable then!