Re: [PATCH] mm: page_ext: validate section in for_each_page_ext iterator

["David Hildenbrand (Arm)" <david@kernel.org>]

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@kernel.org>
Date: Fri, 19 Jun 2026 09:58:46 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 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;
-- 
2.43.0


-- 
Cheers,

David