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

[Luiz Capitulino <luizcap@redhat.com>]

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?

> 
>>
>> Add a valid-section check before looking up the next page_ext
>> so the iterator stops cleanly at section boundaries.
>>
>> Fixes: 9039b9096ea2 ("mm: page_owner: use new iteration API")
> 
> If the problem is real, we'd want to CC stable.
> 
>> Signed-off-by: Ketan Kishore <ketan.kishore@oss.qualcomm.com>
>> ---
>>   mm/page_ext.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/mm/page_ext.c b/mm/page_ext.c
>> index e2e92bd27ebd..74067ea740fe 100644
>> --- a/mm/page_ext.c
>> +++ b/mm/page_ext.c
>> @@ -253,6 +253,18 @@ static struct page_ext *lookup_page_ext(const struct page *page)
>>   {
>>   	unsigned long pfn = page_to_pfn(page);
>>   	struct mem_section *section = __pfn_to_section(pfn);
>> +
>> +	/*
>> +	 * section can be NULL when the page_ext iterator's for-loop increment
>> +	 * computes a PFN one step beyond the last registered section. This
>> +	 * occurs because pfn_to_page() uses __nr_to_section() which succeeds
>> +	 * for unregistered sections that share a root array with registered
>> +	 * sections,while __pfn_to_section() returns NULL for them.
>> +	 *
>> +	 */
>> +	if (!section)
>> +		return NULL;
>> +
>>   	struct page_ext *page_ext = READ_ONCE(section->page_ext);
> 
> In addition to what Zi says, which will end up as:
> 
> 	if (!section)
> 		return NULL;
> 	page_ext = READ_ONCE(section->page_ext);
> 
> 
> drop that comment, not required. And likely best to just add a unlikely(),
> because this doesn't usually happen.
>