Re: [PATCH] mm: page_ext: validate section in for_each_page_ext iterator
From: Ketan Kishore
Date: Fri Jun 19 2026 - 16:36:18 EST
On 6/19/2026 1:30 PM, David Hildenbrand (Arm) wrote:
On 6/19/26 04:38, Luiz Capitulino wrote:
On 2026-06-18 05:04, David Hildenbrand (Arm) wrote:
On 6/17/26 17:09, Ketan wrote:
The page_ext iteration API do not validate if the PFN still
belongs to a valid section while advancing the iterator.
When dynamically adding memory in hotplug path, it can lead
to a NULL pointer dereference during page_ext_lookup at the
boundary of the last valid section.
[ 14.555124][ T846] Call trace:
[ 14.555125][ T846] lookup_page_ext+0x6c/0x108 (P)
[ 14.555127][ T846] page_ext_lookup+0x30/0x3c
[ 14.555129][ T846] +0x11c/0x260
[ 14.571201][ T846] __free_pages_ok+0x5e8/0x8e0
[ 14.571204][ T846] __free_pages_core+0x78/0xf0
[ 14.571206][ T846] generic_online_page+0x14/0x24
[ 14.597782][ T846] online_pages+0x178/0x30c
[ 14.597784][ T846] memory_block_change_state+0x284/0x32c
[ 14.597787][ T846] memory_subsys_online+0x4c/0x64
[ 14.597789][ T846] device_online+0x88/0xb0
[ 14.597791][ T846] online_memory_block+0x30/0x40
[ 14.597793][ T846] walk_memory_blocks+0xac/0xe8
[ 14.597794][ T846] add_memory_resource+0x280/0x298
[ 14.656161][ T846] add_memory+0x60/0x98
So, we do allocate the page_ext in memory_notify(MEM_GOING_ONLINE) -> ...
page_ext_callback() -> online_page_ext().
Which makes sure that all page_ext is actually allocated (for the full section).
So when we later end up in generic_online_page(), the page_ext should be there
and the memory section should be valid.
How can online_pages(), which onlines memory part of present memory sections,
online (free) memory that suddenly does not belong to a present memory section?
Something doesn't make sense here.
Ketan, would you please share the full OOPs message and maybe tell us
how you reproduce this issue?
Ah, I think I figured it out.
for_each_page_ext() does a "__page_ext = page_ext_iter_next(&__iter)" and the end.
That is the real problem and should be spelled out in the patch description.
Likely we should just handle that in the iterator, looking up page-ext for something
that doesn't exist is weird.
From ce41607a4e4c1d872b6d3050da2e24d41a30d822 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
Date: Fri, 19 Jun 2026 09:58:46 +0200
Subject: [PATCH] tmp
Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
---
include/linux/page_ext.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 61e876e255e8..468c074582e1 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -112,6 +112,7 @@ static inline struct page_ext *page_ext_next(struct page_ext *curr)
struct page_ext_iter {
unsigned long index;
+ unsigned long pgcount;
unsigned long start_pfn;
struct page_ext *page_ext;
};
@@ -120,15 +121,19 @@ struct page_ext_iter {
* page_ext_iter_begin() - Prepare for iterating through page extensions.
* @iter: page extension iterator.
* @pfn: PFN of the page we're interested in.
+ * @pgcount: number of page_ext entries to iterate
*
* Must be called with RCU read lock taken.
*
* Return: NULL if no page_ext exists for this page.
*/
static inline struct page_ext *page_ext_iter_begin(struct page_ext_iter *iter,
- unsigned long pfn)
+ unsigned long pfn, unsigned long pgcount)
{
+ VM_WARN_ON_ONCE(!pgcount);
+
iter->index = 0;
+ iter->pgcount = pgcount;
iter->start_pfn = pfn;
iter->page_ext = page_ext_lookup(pfn);
@@ -149,8 +154,9 @@ static inline struct page_ext *page_ext_iter_next(struct page_ext_iter *iter)
if (WARN_ON_ONCE(!iter->page_ext))
return NULL;
+ if (++iter->index >= iter->pgcount)
+ return NULL;
- iter->index++;
pfn = iter->start_pfn + iter->index;
if (page_ext_iter_next_fast_possible(pfn))
@@ -183,9 +189,9 @@ static inline struct page_ext *page_ext_iter_get(const struct page_ext_iter *ite
* IMPORTANT: must be called with RCU read lock taken.
*/
#define for_each_page_ext(__page, __pgcount, __page_ext, __iter) \
- for (__page_ext = page_ext_iter_begin(&__iter, page_to_pfn(__page));\
- __page_ext && __iter.index < __pgcount; \
- __page_ext = page_ext_iter_next(&__iter))
+ for (__page_ext = page_ext_iter_begin(&__iter, page_to_pfn(__page), __pgcount);\
+ __page_ext; \
+ __page_ext = page_ext_iter_next(&__iter))
#else /* !CONFIG_PAGE_EXTENSION */
struct page_ext;
Sure David, your above understanding is correct.
Thankyou for the suggested alternative fix, fixing the iterator itself seems to be a better approach than having the section check in lookup_page_ext.
Thanks & Regards,
Ketan