Re: [PATCH] mm/util: don't read __page_2 for order-1 folios in snapshot_page()

[Aboorva Devarajan]

On Wed, 2026-07-08 at 09:31 +0100, Lorenzo Stoakes wrote:
> On Wed, Jul 08, 2026 at 10:10:33AM +0200, David Hildenbrand (Arm) wrote:
> > On 7/8/26 03:52, Aboorva Devarajan wrote:
> > > snapshot_page() reconstructs a folio from a struct page.  After copying
> > > the head and __page_1 it reads __page_2 whenever the folio has more than
> > > one page:
> > > 
> > > 	if (nr_pages > 1)
> > > 		memcpy(&ps->folio_snapshot.__page_2, &foliop->__page_2,
> > > 		       sizeof(struct page));
> > > 
> > > __page_2 is the folio's third struct page, so it is part of the folio
> > > only for order >= 2 (nr_pages > 2).  For an order-1 folio (exactly two
> > > pages) __page_2 is not part of the folio at all, it is the struct page
> > > of the following pfn.
> > > 
> > > When such an order-1 head sits in the last struct page slots of a
> > > populated section whose neighbouring section is absent (a memory hole),
> > > __page_2 falls into the next section's unpopulated vmemmap and the
> > > read oopses.
> > > 
> > > Observed on a 22 TB ppc64le LPAR during DLPAR memory remove, on the page
> > > isolation dump path:
> > > 
> > > 	offline_pages -> start_isolate_page_range -> isolate_single_pageblock
> > > 	  -> set_migratetype_isolate -> dump_page -> __dump_page -> snapshot_page
> > > 
> > > 	NIP   = snapshot_page+264  (ld of __page_2)
> > > 	r4    = foliop = head = 0xc00c0005a03fff80
> > > 	DAR   = r4 + 0x88     = 0xc00c0005a0400008   (unmapped)
> > > 	DSISR = 0x40000000                           (no translation)
> > > 
> > > The faulting head was a free page that still carried PG_head with
> > > _nr_pages == 2; its __page_2 is the first entry of the absent section.
> > > 
> > > It is also reproducible deterministically in a VM by placing an order-1
> > > folio in the last slots of a populated section adjacent to a hole
> > > (memmap=nnM$ssM) and calling dump_page() on it.
> > > 
> > > Only read __page_2 for order >= 2 folios (nr_pages > 2).
> > 
> > Hi!
> > 
> > Can you shorten that a bit? It's rather trivial, really.
> > 
> > "snapshot_page() currently reads __page_2 after checking nr_pages > 1, whereby
> > we really should only do so for nr_pages > 2. Let's fix that to avoid reading
> > memmap that doesn't exist (e.g., vmemmap hole)
> > 
> > 
> > Observed on a 22 TB ppc64le LPAR during DLPAR memory remove ...
> > "
> > 
> > > 
> > > Fixes: 31a31da8a618 ("mm: move _pincount in folio to page[2] on 32bit")
> > > Cc: stable@xxxxxxxxxxxxxxx # v6.15+
> > > Reported-by: Sourabh Jain <sourabhjain@xxxxxxxxxxxxx>
> > > Signed-off-by: Aboorva Devarajan <aboorvad@xxxxxxxxxxxxx>
> > > ---
> > >  mm/util.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/mm/util.c b/mm/util.c
> > > index af2c2103f0d95..b3d48a05e6d82 100644
> > > --- a/mm/util.c
> > > +++ b/mm/util.c
> > > @@ -1353,7 +1353,13 @@ void snapshot_page(struct page_snapshot *ps, const struct page *page)
> > >  	if (ps->idx < MAX_FOLIO_NR_PAGES) {
> > >  		memcpy(&ps->folio_snapshot, foliop, 2 * sizeof(struct page));
> > >  		nr_pages = folio_nr_pages(&ps->folio_snapshot);
> > > -		if (nr_pages > 1)
> > > +		/*
> > > +		 * __page_2 is the folio's third struct page and is part of the
> > > +		 * folio only for order >= 2 (nr_pages > 2).  For an order-1
> > > +		 * folio it is not part of the folio and may fall into an
> > > +		 * adjacent, possibly absent, section.
> > > +		 */
> > 
> > No need for the comment, really, this is rather trivial.
> > 
> > > +		if (nr_pages > 2)
> > >  			memcpy(&ps->folio_snapshot.__page_2, &foliop->__page_2,
> > >  			       sizeof(struct page));
> > >  		set_ps_flags(ps, foliop, page);
> > 
> > 
> > With a condensed patch description and the comment dropped
> > 
> > Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> > 
> > Thanks!
> > 
> > --
> > Cheers,
> > 
> > David
> 
> Agree with everything David said :)
> 
> Patch looks good with changes David suggested applied, so feel free to add my
> tag to v2 alongside David's:
> 
> Reviewed-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> 
> Cheers, Lorenzo


Hi David, Lorenzo,

Thanks for the review.

I've incorporated the suggested changes in v2:
https://lore.kernel.org/all/20260708201954.686111-1-aboorvad@xxxxxxxxxxxxx/

Regards,
Aboorva