The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Kiryl Shutsemau <kas@kernel.org>
To: "Lorenzo Stoakes (ARM)" <ljs@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 17:32:10 +0100	[thread overview]
Message-ID: <alUNiWcygLd4rqBo@thinkstation> (raw)
In-Reply-To: <alTq4V50767L-s5H@lucifer>

On Mon, Jul 13, 2026 at 04:54:27PM +0100, Lorenzo Stoakes (ARM) wrote:
> 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 think the userland side is fixable too.

My first thought was to walk only VMA-backed ranges, which would make
the mmap read lock sufficient: munmap() detaches VMAs under the write
lock before downgrading, and free_pgtables() only frees tables
exclusively covering the detached range.

But that is an unnecessary limitation on MMU_GATHER_RCU_TABLE_FREE
architectures -- which is every architecture with generic ptdump
support. There, user page table freeing is already RCU-deferred; it is
what GUP-fast relies on to walk user page tables blindly with no mmap
lock. khugepaged retracting PTE tables under the pmd lock is likewise
covered by pte_free_defer().

So the downgraded munmap() teardown you mention is already safe to race
with -- for a walker inside an RCU read-side section. The write lock is
only needed today because ptdump walks outside of one, so the deferred
freeing does nothing for it.

That would make the end state: the whole of ptdump -- userland, efi_mm
and kernel ranges -- walks under RCU with no mmap lock taken at all,
and the kernel-side freeing conversion we discussed is the only
missing piece.

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

I think you forgot to paste [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).

No argument, let's do it as a follow-up.

I will give the current patchset a proper review.

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

You are right. CPA allocates the split tables with bare
pagetable_alloc(), so ptdesc_test_kernel() is false and collapse frees
them via __pagetable_free() directly. Note this also means they skip
the IOMMU SVA KVA invalidation that ASYNC_KERNEL_PGTABLE_FREE is there
for -- that looks like a bug today, independent of ptdump.

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

Sure, call_rcu().

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

Since the walker only reads, I think it is enough to re-descend from
the pgd with fresh READ_ONCE() at each level after dropping RCU --
whatever level got promoted in the meantime, we see either the new leaf
or the old table. But agreed this is the part that needs the most care.

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

For a walker that only reads, I don't think staleness is a problem as
long as frees are RCU-deferred: the walker can only reach a table via
an entry it read within its RCU section, and freeing requires unlinking
that entry first, so the grace period covers any walker that saw the
old pointer. Stale leaf contents are harmless for a dumper. It does
become a problem for anything that dereferences through leaf entries --
agreed that the general case needs more care than ptdump does.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

      reply	other threads:[~2026-07-13 16:32 UTC|newest]

Thread overview: 14+ 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 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 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)
2026-07-13 16:32     ` Kiryl Shutsemau [this message]

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=alUNiWcygLd4rqBo@thinkstation \
    --to=kas@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=liam@infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.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