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

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

[Aboorva Devarajan]

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

Fixes: 31a31da8a618 ("mm: move _pincount in folio to page[2] on 32bit")
Cc: stable@vger.kernel.org # v6.15+
Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
---
 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.
+		 */
+		if (nr_pages > 2)
 			memcpy(&ps->folio_snapshot.__page_2, &foliop->__page_2,
 			       sizeof(struct page));
 		set_ps_flags(ps, foliop, page);
-- 
2.54.0

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

[David Hildenbrand (Arm)]

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@vger.kernel.org # v6.15+
> Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> ---
>  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@kernel.org>

Thanks!

-- 
Cheers,

David

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

[Lorenzo Stoakes]

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@vger.kernel.org # v6.15+
> > Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> > ---
> >  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@kernel.org>
>
> 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@kernel.org>

Cheers, Lorenzo

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@vger.kernel.org # v6.15+
> > > Reported-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> > > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> > > ---
> > >  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@kernel.org>
> > 
> > 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@kernel.org>
> 
> 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@linux.ibm.com/

Regards,
Aboorva