Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: luto@kernel.org
Cc: linux-mm@kvack.org, luto@amacapital.net,
	"David Hildenbrand (Red Hat)" <david@kernel.org>,
	Qi Zheng <zhengqi.arch@bytedance.com>,
	Liam Howlett <liam.howlett@oracle.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Michal Hocko <mhocko@suse.com>, Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	stable@vger.kernel.org, dfoxfranke@gmail.com
Subject: Re: [PATCH] Fix incorrect flush address in direct page table reclaim
Date: Mon, 3 Aug 2026 18:30:32 -0700	[thread overview]
Message-ID: <20260803183032.7cc1fb1007d309c2431855ea@linux-foundation.org> (raw)
In-Reply-To: <20260804003708.49830-1-luto@kernel.org>

On Mon,  3 Aug 2026 17:37:08 -0700 luto@kernel.org wrote:

> From: Andy Lutomirski <luto@kernel.org>
> 
> When zap_pte_range reclaims a page table, it does:
> 
>     pte_free_tlb(tlb, pmd_pgtable(pmdval), addr);
> 
> and this is unconditionally wrong: if this code executes, addr *always*
> points one past the end of the range covered by the table.  The addr
> parameter is used to flush the TLB (really the paging-structure-cache)
> to drop references to the to-be-freed table, and any architecture that
> cares about the parameter will flush the wrong address.  (But they'll
> still free the correct page).
> 
> I think it's worth contemplating why the kernel works at all.
> 
> If we hit the offending line of code, we will first clear the PMD entry
> (line 1954, zap_empty_pte_table), then we will issue pending flushes if
> force_flush is set (tlb_flush_mmu_tlbonly(tlb)), then we will skip the
> retry on line 1979 (phew!), and then we will do the offending
> pte_free_tlb call.  *Or* we will clear the PMD entry immediately before
> pte_free_tlb (line 1983, zap_pte_table_if_empty).
> 
> If we have any pending flushes (i.e. we actually zapped any last-level
> entries) at the time we clear the PMD entry, then the flush really ought
> to flush all references to the table (Linus certainly seems to think it
> will on all architectures [0]).
> 
> The condition under which we have no accumulated flushes at the time of
> the clear is very complex (the whole zap_pte_range function has absurdly
> complex control flow).  If we do hit the bad case, then we will end up
> clearing the PMD entry after the last time the range is flushed, and any
> CPU is free to cache a reference to the (empty) page table.  If this
> happens due to an ordinary read or write, it would segfault, so it would
> be rare.  But the cache could be speculatively filled as well.  Then
> we'll flush the wrong address and then free and possibly reuse the
> table.
> 
> On x86, even flushing the wrong address works on non-KPTI Intel systems
> because INVLPG flushes *all* paging-structure-caches, not just the ones
> for the target address.  But INVPCID does not, and flush_tlb_one_user
> will use INVPCID if it's available.  And then we're toast.  AMD systems
> are more susceptible: we set the EFER.TCE bit, which makes even INVLPG
> only flush the target address.
> 
> P.S. IMO zap_pte_range is a mess.  The control flow is excessively
> complex.  The direct_reclaim variable itself has a confused meaning --
> for the first part of the function it means, approximately, "we should
> free the table if can_reclaim_pt".  But, later on, it means "we ALREADY
> reclaimed the table".  And the goto retry on line 1979 is IMO just
> asking for trouble if the condition ever changes such that it might
> happen after clearing the PMD.
> 
> I think this might fix an issue in ripgrep reported here:
> https://github.com/BurntSushi/ripgrep/issues/3494

Huh, cool.  Very recently I was scratching my head at Daniel's ripgrep
report.  Upon which he obviously did a ton of work (many thanks).

	https://github.com/dfoxfranke/ripgrep-3494-analysis

Daniel also pointed a finger at 4c640eb4181c ("mm: move pte table
reclaim code to memory.c").

The manifestation is that a tiny race window causes the zero page to
magically appear where an anon page was expected.

Anyway, I see Daniel is testing this patch.  Please let us know the
result.  (I think I found his email address).

I can't find an email address for BurntSushi.




  reply	other threads:[~2026-08-04  1:30 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04  0:37 [PATCH] Fix incorrect flush address in direct page table reclaim luto
2026-08-04  1:30 ` Andrew Morton [this message]
2026-08-04  2:18   ` Andy Lutomirski
     [not found] ` <0adaae9e-7086-4213-811e-ba6e4181e145@kernel.org>
2026-08-04 15:54   ` Linus Torvalds
2026-08-05  6:04     ` David Hildenbrand (Arm)
2026-08-05 15:02       ` Linus Torvalds
2026-08-05 15:06         ` David Hildenbrand (Arm)
     [not found] ` <313133db-3b94-4a63-8bcd-21952ff77e41@kernel.org>
     [not found]   ` <8cf8eaca-fc04-4a68-bbc8-50ecdbf96382@linux.dev>
     [not found]     ` <3d3aa670-76bc-4df3-9c36-609c14354e9e@kernel.org>
2026-08-04 13:43       ` Andy Lutomirski
2026-08-04 17:05         ` David Hildenbrand (Arm)
2026-08-04 18:34       ` David Hildenbrand (Arm)

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=20260803183032.7cc1fb1007d309c2431855ea@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=dfoxfranke@gmail.com \
    --cc=liam.howlett@oracle.com \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=zhengqi.arch@bytedance.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