Re: [PATCH v3 2/2] mm/page_isolation: guard compound_order() against racing

[Qi Xi <xiqi2@huawei.com>]

Thanks for raising this. I checked the path: PageHuge(head) resolves to
folio_test_hugetlb(), which reads page_type via data_race(). I don't think
it's the same issue this series targets:

- It never touches page->flags or PF_SECOND — folio_test_hugetlb() reads
page_type on the head page via FOLIO_TYPE_OPS, a separate "page type" mechanism,
so neither folio_flags() nor its VM_BUG_ON_PGFLAGS assertions are on its path.
- The only shift is a fixed >> 24 on a u32, not a variable 1 << order,
so it can't produce the shift-out-of-bounds UBSAN this series fixes.

So no extra stabilization seems needed for PageHuge() itself.
Happy to add it if you see a case I missed.

Qi

On 28/08/2026 11:47, Andrew Morton wrote:
> On Tue, 25 Aug 2026 20:05:49 +0800 Qi Xi <xiqi2@huawei.com> wrote:
>
>> The PageCompound branch reads compound_head() without holding a reference.
>> A racing split or free can cause compound_head() to return a stale pointer,
>> and compound_nr() reads the order from that stale head, leading to
>> out-of-range shifts and making the skip distance meaningless.
>>
>> Read the order explicitly with compound_order() and validate it is within
>> MAX_FOLIO_ORDER before shifting. Also verify the derived head_pfn against
>> the legitimate pfn: the head must not be past pfn, must be aligned to
>> nr_pages, and pfn must fall within the compound page. Bail out with
>> -EBUSY if any check fails.
>>
>> ...
>>
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -418,10 +418,28 @@ static int isolate_single_pageblock(unsigned long boundary_pfn,
>>   		if (PageCompound(page)) {
>>   			struct page *head = compound_head(page);
>>   			unsigned long head_pfn = page_to_pfn(head);
>> -			unsigned long nr_pages = compound_nr(head);
>> +			unsigned int order = compound_order(head);
>> +			unsigned long nr_pages;
>> +
>> +			/* compound_order() is racy. Cap it at MAX_FOLIO_ORDER. */
>> +			if (order > MAX_FOLIO_ORDER)
>> +				goto failed;
>> +
>> +			nr_pages = 1UL << order;
>> +
>> +			/*
>> +			 * compound_head() is also racy, so the derived head_pfn
>> +			 * needs additional checks to make sure it is valid.
>> +			 * Otherwise, just fail the check. pfn comes from
>> +			 * __first_valid_page() as a legitimate PFN, so use it to
>> +			 * check head_pfn.
>> +			 */
>> +			if (head_pfn > pfn || !IS_ALIGNED(head_pfn, nr_pages) ||
>> +			    pfn - head_pfn >= nr_pages)
>> +				goto failed;
>>   
>>   			if (head_pfn + nr_pages <= boundary_pfn ||
>> -			    PageHuge(page)) {
>> +			    PageHuge(head)) {
> Sashiko suggests that this PageHuge() test suffers the same issue?
>
> 	https://sashiko.dev/#/patchset/20260825120549.966271-1-xiqi2@huawei.com
>
>>   				pfn = head_pfn + nr_pages;
>>   				continue;
>>   			}
>> -- 
>> 2.33.0
>>