mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Zi Yan <ziy@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 David Hildenbrand <david@kernel.org>,
	Rik van Riel <riel@surriel.com>,
	 "Liam R. Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	 Harry Yoo <harry@kernel.org>, Jann Horn <jannh@google.com>,
	Lance Yang <lance.yang@linux.dev>,
	 Kyle Zeng <kylebot@openai.com>,
	stable@vger.kernel.org, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level
Date: Tue, 13 Oct 2026 15:29:44 +0100	[thread overview]
Message-ID: <as49h2Zgs2wtkfmJ@gremlin> (raw)
In-Reply-To: <9CC1C4F0-1C73-4AA4-8D1C-28DB024E2221@nvidia.com>

On Mon, Oct 12, 2026 at 04:37:34PM -0400, Zi Yan wrote:
> On 12 Oct 2026, at 11:22, Lorenzo Stoakes (ARM) wrote:
>
> > On Mon, Oct 12, 2026 at 10:33:34AM -0400, Zi Yan wrote:

> > }
>
> Yes.
>
> My wording was not precise.
>
> zap_pmd_range() doesn’t take the PTL since it sees pmd_none(), while
> try_to_unmap_one() takes the PTL.

Yeah, this is nasty.

No wonder it took some AI analysis to find this :)

> > That bit isn't special to MREMAP_DONTUNMAP.
>
> Right.

But it is a 'very special' flag for mremap... :)

The # of bugs that thing has produced...

> > That set pmd could also UAF on the PMD page table in theory couldn't it?
>
> No, in __discard_anon_folio_pmd_locked(), the caller holds anon_vma read lock;
> when mremap() or munmap() calls free_pgtables() to get rid of page tables,
> they first call unlink_anon_vmas() and get blocked by the aforementioned read
> lock. set_pmd_at() is called while free_pgtables() is blocked, so no UAF here.

Ah ok thanks. Yeah the rmap lock is held throughout that.

Hey - when I eliminate the rmap lock things are going to be even more fun ;)

>
> >
> > It's all really horrible.
>
> And kills my brain.

Yeah this hurt my head too.

I actually (rare in 2026) only got AI to chime in late in my analysis, and while
it helped confirm/deny some stuff it wasn't even that helpful in avoiding the
head... f..un.

> >>> So unmap_vmas() doesn't correctly populate the mmu_gather struct for the
> >>> range, meaning that the underlying page isn't freed and the refcount,
> >>> mapcount etc. are all left as they were?
> >>
> >> Yes to the underlying page isn’t freed.
> >
> > The refcount and mapcount etc. would be correct? I think the refcount would be
> > such that it permanently leaks and the mapcount would be incorrect or only
> > accidentally correct?
>
> refcount and mapcount would be wrong in the munmap case. Both are correct
> in MREMAP_DONTUNMAP case, since the folio is still mapped in the source VMA
> but its anon_vma says it belongs to dst VMA. When dst VMA is unmapped,
> the anon_vma is freed, but the folio is still mapped and its ->mapping
> points to a freed anon_vma. A rmap code checking folio->mapping is a UAF.

OK I _think_ you can only have a destination that either has the same anon_vma
or is unfaulted (so gets given the source anon_vma) so yeah, the anon_vma is
still right but is pointing at the destination VMA, while still being mapped in
the source one :)))

MREMAP_DONTUNMAP really is the gift that keeps on giving.

> > They seem like they are existing bugs effectively in pmdp_invalidate()? Or stuff
> > that we would want to fix anyway I guess?
>
> SPARC’s thp_pte_count might cause other issues, since its code only considers
> huge pmd splitting. powerpc’s page table check might have an issue.

Such fun... lord.

> >> I can remove the dead code if you think that is better.
> >
> > No, if you plan to reinstate it (soon!) then that's fine, but please
> > explain that you're intentionally doing that in the commit message please!
>
> Sure. I will try to come up with a concise but comprehensive message to cover
> both the issue and why pmdp_invalidate() does not work itself.

Great thanks, I saw the v2 and it all LGTM!

--
Cheers, Lorenzo

      reply	other threads:[~2026-10-13 14:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-10  2:08 Zi Yan
2026-10-10  2:14 ` Kyle Zeng
2026-10-10  2:28 ` Lance Yang
2026-10-12 10:25 ` David Hildenbrand (Arm)
2026-10-12 12:07 ` Lorenzo Stoakes (ARM)
2026-10-12 14:33   ` Zi Yan
2026-10-12 14:39     ` David Hildenbrand (Arm)
2026-10-12 15:22       ` Lorenzo Stoakes (ARM)
2026-10-12 15:22     ` Lorenzo Stoakes (ARM)
2026-10-12 20:37       ` Zi Yan
2026-10-13 14:29         ` Lorenzo Stoakes (ARM) [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=as49h2Zgs2wtkfmJ@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --cc=jannh@google.com \
    --cc=kylebot@openai.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@surriel.com \
    --cc=stable@vger.kernel.org \
    --cc=vbabka@kernel.org \
    --cc=ziy@nvidia.com \
    /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

all inboxes | Powered by JetHome®