All of lore.kernel.org
 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 [PATCH] mm/rmap: don't discard lazyfree THPs at PMD level 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.