Re: [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning

["Zi Yan" <ziy@nvidia.com>]

On Wed Aug 19, 2026 at 3:55 AM EDT, Qi Xi wrote:
>
> On 19/08/2026 03:14, Zi Yan wrote:
>> On Tue Aug 18, 2026 at 7:28 AM EDT, Qi Xi wrote:
>>> A contig-range allocation racing with buddy allocation on the adjacent
>>> pageblock can trigger:
>>>
>>>   UBSAN: shift-out-of-bounds in mm/page_isolation.c:393:15
>>>   shift exponent -749042176 is negative
>>>   Call trace:
>>>    isolate_single_pageblock
>>>    start_isolate_page_range
>>>    alloc_contig_frozen_range_noprof
>>>    alloc_contig_range_noprof
>>>
>>> isolate_single_pageblock() first calls set_migratetype_isolate() with
>>> zone->lock held, which marks the pageblock MIGRATE_ISOLATE and moves any
>>> free page straddling the boundary out of the way.  Once the lock is
>>> dropped, it scans the MAX_ORDER_NR_PAGES-aligned window [start_pfn,
>>> boundary_pfn) locklessly, only to skip the free pages already handled
>>> above and to detect in-use pages straddling the boundary.  Since this
>>> scan only reads page state to decide how far to skip and returns -EBUSY
>>> on a straddling in-use page, it does not take the lock.
>>>
>>> The window also covers the adjacent pageblock, whose free pages stay on
>>> the normal movable/CMA freelist and can be allocated concurrently.  So
>>> after the scan observes PageBuddy(page), another CPU can allocate the
>>> page, leaving a stale value in page->private that makes "1 << order" shift
>>> out of range.
>>>
>>> Use buddy_order_unsafe() to read the order exactly once (READ_ONCE), and
>>> guard the shift with an order <= MAX_PAGE_ORDER check so it is never
>>> performed with a bogus value.
>>>
>>> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
>>> Signed-off-by: Qi Xi <xiqi2@huawei.com>
>>> ---
>>>   mm/page_isolation.c | 13 ++++++++-----
>>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>> The fix makes sense to me.
>> Please Cc stable for this. Thanks.
>>
>> Sashiko has two more concerns:
>>
>> 1. order can still be bogus even if it is within MAX_PAGE_ORDER.
>>      a. it can trigger VM_WARN_ON_ONCE();
>>      b. it can skip arbitrary pages and miss an in-use compound page.
>>
>> For 1a, we can remove VM_WARN_ON_ONCE() and just fail
>> isolate_single_pageblock() if PageBuddy crosses the boundary, since
>> pageblock_isolate_and_move_free_pages() handles it.
>>
>> For 1b, after we makes the change in 1a, PageBuddy skip cannot land us
>> beyond the boundary, so the concern is gone.
>>
>>
>> 2. In PageCompound() branch, a bogus nr_pages can also cause the
>> shift-out-of-bounds.
>>
>> It will need a different fix.
>>
>>
>> Hi Qi,
>>
>> Do you mind fixing 1a along with this patch?
>>
>> For this patch alone,
>>
>> Reviewed-by: Zi Yan <ziy@nvidia.com>
> Thanks for the review and suggestions.  I'll address them in a
> follow-up: fold concern 1a into a v2, and send a separate patch
> for concern 2.  Both will carry Cc: stable.

Thanks.

For concern 2, I used Claude to do some investigation and come up with
the code below, feel free to use it as a reference.


if (PageCompound(page)) {
	struct page *head = compound_head(page);
	unsigned long head_pfn = page_to_pfn(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
     * legitmate 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(head)) {
		pfn = head_pfn + nr_pages;
		continue;
	}

	/*
	 * These pages are movable too, but they're
	 * not expected to exceed pageblock_order.
	 *
	 * Let us know when they do, so we can add
	 * proper free and split handling for them.
	 */
	VM_WARN_ON_ONCE_PAGE(PageLRU(page), page);
	VM_WARN_ON_ONCE_PAGE(page_has_movable_ops(page), page);

	goto failed;
}



-- 
Best Regards,
Yan, Zi