BPF List
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Kiryl Shutsemau <kas@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 Suren Baghdasaryan <surenb@google.com>,
	"Liam R. Howlett" <liam@infradead.org>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	 David Hildenbrand <david@kernel.org>,
	Mike Rapoport <rppt@kernel.org>, Michal Hocko <mhocko@suse.com>,
	 Uladzislau Rezki <urezki@gmail.com>,
	Toshi Kani <toshi.kani@hpe.com>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	 Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@kernel.org>,
	 Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Will Deacon <will@kernel.org>, Dev Jain <dev.jain@arm.com>,
	Ryan Roberts <ryan.roberts@arm.com>,
	 David Carlier <devnexen@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	 bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	stable@vger.kernel.org,
	 syzbot+fd95a72470f5a44e464c@syzkaller.appspotmail.com
Subject: Re: [PATCH mm-hotfixes v2 0/4] mm: fix UAF caused by race between ptdump and vmap pgtable freeing
Date: Mon, 13 Jul 2026 16:54:27 +0100	[thread overview]
Message-ID: <alTq4V50767L-s5H@lucifer> (raw)
In-Reply-To: <alTn7NguEW_4bodu@thinkstation>

On Mon, Jul 13, 2026 at 02:32:09PM +0100, Kiryl Shutsemau wrote:
> On Sun, Jul 12, 2026 at 11:42:23AM +0100, Lorenzo Stoakes wrote:
> > This series addresses the issue by having the vmap huge promotion
> > logic acquire the mmap read lock while both setting the huge page
> > table entry and freeing the prior leaf page table.
>
> Hi Lorenzo,
>
> Before we settle on the mmap lock scheme, have you considered handling
> this the way GUP-fast handles page table freeing -- RCU-defer the free
> and make ptdump a lockless walker?

Overall I like the idea of eventually moving this to _all_ being
RCU-free-able :)

BUT... I don't like it as a fix for this bug that has to be backported.

I think there's a lot of subtleties to worry about and I don't want to
worry about having to maintain tht as a backport.

But also, we have an unfortunate situation with ptdump where it also walks
userland ranges on x86 (and ranges for efi_mm on both x86 and arm64).

And for userland ranges you have to have the mmap write lock to exclude a
downgraded mmap read lock munmap() operation (which gives rise to the weird
inversion you mention).

So the walker still has to take the write lock in this case.

I spoke a bit about it overall at [0].

We already have this mmap lock convention as a requirement for kernel
ranges, and it was being violated by CPA and vmalloc.

So I'd prefer we keep this as the proximate fix to solve the bug here, and
then revisit this later (alongside moving to RCU page table freeing
_overall_, though one doesn't have to block the other).

I believe Suren is looking at this (and I'm happy to look if Suren is tied
up with other things also!)

>
> The locking here is inverted from what one would normally expect
> (walker takes the write lock, mutators take read locks, mutators safe
> against each other only by range ownership). It works, but it is

Yeah :) this is a direct result of the munmap() thing mentioned above
(because ptdump can read non-kernel ranges).

Because you _have_ to take the mmap write lock there, so you _may as well_
exclude with a read lock and then get to do an mmap read trylock as an
added bonus on vmap huge page promotion.

> subtle, it is what produced the arm64 deadlock and the ifdeffery, and
> it depends on every current and future freeing site remembering the
> rule -- patch 3 exists because two walkers did not fit the scheme.

Well everybody who does things with shared resources should be aware of
locking requirements.

>
> The free side looks cheap: kernel page table freeing already funnels
> through pagetable_free_kernel(), which already has a deferred path
> (used for IOMMU SVA). Adding a grace period there -- synchronize_rcu()
> in the worker, amortized over the batch -- covers every freeing site
> by construction.

Well I did want to say btw that CPA doesn't actually mark the page tables
as kernel, was going to chase up with something on that when I got a chance
:)

synchronize_rcu() is a very bug hammer, you can just use call_rcu() (and
the ptdesc already has an rcu_head I think).


>
> On the walk side, nothing on the ptdump path can sleep -- the pagewalk
> core only allocates for install_pte ops, kernel PTE level uses
> pte_offset_kernel(), and the arch note_page() implementations are
> seq_printf()/printk() into a preallocated buffer. So the walk could run
> under rcu_read_lock() as is. The real work is bounding the read-side
> sections: a full walk can take dozens of seconds on a KASAN kernel per
> the comment in mm/ptdump.c, so it would need to drop RCU and
> cond_resched() periodically, re-descending from the top. Note we
> currently hold the init_mm mmap write lock across those same dozens of
> seconds, and this series makes that load-bearing: during a long walk
> every promotion trylock fails, silently degrading vmalloc to small
> pages, and CPA collapse blocks.
>

Yeah it all sucks, but you're really making my point that this is better to
revisit and consider carefully rather than doing as a fix.

> That would give us: no inverted locking, no arm64 deadlock possibility
> (your patch 4 stands on its own), the patch 3 walkers covered
> structurally rather than by locking init_mm as well, and ptdump
> invisible to production paths.
>
> Given the live UAF, this could also be a follow-up rather than a
> respin. But I would like to hear whether you see a fatal flaw in the
> approach first.

Yes - as above obviously I'd definitely prefer to defer this to a
follow-up.

The main concern for ptdump is properly slicing its walk to avoid
RCU stalls.

Keeping in mind that vmap can (in theory) span PUDs and even P4Ds it
becomes a bit tricky.

But also I worry about whether the entries in the page table will actually
be valid at the point the walker reads them.

For vmap/CPA pretty much yes they are, but if something was to actually
unmap them in future then that might no longer be the case. RCU will only
guarantee that the page tables stick around, not that they contain anything
valid.

In userland a lot of this is solved already by the leaf page table entry's
PTL.

Kernel page tables are a bit of a weird case, because mostly we solve
things through exclusive owners, ptdump being the weirdo outlier.

Anyway TL;DR is that - I think we need to be careful and think this through
and make changes across the board for all kernel page table users and
update the page walkers.

I'm happy to spin up an RFC for this when I get a moment :)

But in general I'd like us to solve the problem as a whole across the
kernel for both userland and kernel page table allocations, ideally!

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

Cheers, Lorenzo

  reply	other threads:[~2026-07-13 15:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12 10:42 [PATCH mm-hotfixes v2 0/4] mm: fix UAF caused by race between ptdump and vmap pgtable freeing Lorenzo Stoakes
2026-07-12 10:42 ` [PATCH mm-hotfixes v2 1/4] mm/vmalloc: acquire init_mm lock on huge vmap to avoid ptdump UAF Lorenzo Stoakes
2026-07-12 10:55   ` sashiko-bot
2026-07-12 13:17   ` Dev Jain
2026-07-13 16:42   ` Kiryl Shutsemau
2026-07-12 10:42 ` [PATCH mm-hotfixes v2 2/4] x86/mm/pat: acquire mmap lock on page table free " Lorenzo Stoakes
2026-07-12 11:01   ` sashiko-bot
2026-07-12 16:53   ` Borislav Petkov
2026-07-12 10:42 ` [PATCH mm-hotfixes v2 3/4] mm/ptdump: always stabilise against page table freeing using init_mm Lorenzo Stoakes
2026-07-13 17:19   ` Kiryl Shutsemau
2026-07-12 10:42 ` [PATCH mm-hotfixes v2 4/4] arm64: remove redundant concurrent ptdump UAF mitigation Lorenzo Stoakes
2026-07-13 11:40   ` Will Deacon
2026-07-13 17:21   ` Kiryl Shutsemau
2026-07-13 13:32 ` [PATCH mm-hotfixes v2 0/4] mm: fix UAF caused by race between ptdump and vmap pgtable freeing Kiryl Shutsemau
2026-07-13 15:54   ` Lorenzo Stoakes (ARM) [this message]
2026-07-13 16:32     ` Kiryl Shutsemau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alTq4V50767L-s5H@lucifer \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=devnexen@gmail.com \
    --cc=hpa@zytor.com \
    --cc=kas@kernel.org \
    --cc=liam@infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mhocko@suse.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=shakeel.butt@linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=syzbot+fd95a72470f5a44e464c@syzkaller.appspotmail.com \
    --cc=tglx@kernel.org \
    --cc=toshi.kani@hpe.com \
    --cc=urezki@gmail.com \
    --cc=vbabka@kernel.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox