All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning
@ 2026-08-18 11:28 Qi Xi
  2026-08-18 19:14 ` Zi Yan
  0 siblings, 1 reply; 4+ messages in thread
From: Qi Xi @ 2026-08-18 11:28 UTC (permalink / raw)
  To: Andrew Morton, Vlastimil Babka
  Cc: Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, linux-mm, linux-kernel, sunnanyong,
	wangkefeng.wang, xiqi2

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(-)

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 32ce8a7d9df3..8aa096f1754d 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -387,13 +387,16 @@ static int isolate_single_pageblock(unsigned long boundary_pfn,
 		}
 
 		if (PageBuddy(page)) {
-			int order = buddy_order(page);
+			unsigned int order;
 
-			/* pageblock_isolate_and_move_free_pages() handled this */
-			VM_WARN_ON_ONCE(pfn + (1 << order) > boundary_pfn);
+			order = buddy_order_unsafe(page);
+			if (likely(order <= MAX_PAGE_ORDER)) {
+				/* pageblock_isolate_and_move_free_pages() handled this */
+				VM_WARN_ON_ONCE(pfn + (1 << order) > boundary_pfn);
 
-			pfn += 1UL << order;
-			continue;
+				pfn += 1UL << order;
+				continue;
+			}
 		}
 
 		/*
-- 
2.33.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning
  2026-08-18 11:28 [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning Qi Xi
@ 2026-08-18 19:14 ` Zi Yan
  2026-08-19  7:55   ` Qi Xi
  0 siblings, 1 reply; 4+ messages in thread
From: Zi Yan @ 2026-08-18 19:14 UTC (permalink / raw)
  To: Qi Xi, Andrew Morton, Vlastimil Babka
  Cc: Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, linux-mm, linux-kernel, sunnanyong,
	wangkefeng.wang

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>


-- 
Best Regards,
Yan, Zi


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning
  2026-08-18 19:14 ` Zi Yan
@ 2026-08-19  7:55   ` Qi Xi
  2026-08-19 14:39     ` Zi Yan
  0 siblings, 1 reply; 4+ messages in thread
From: Qi Xi @ 2026-08-19  7:55 UTC (permalink / raw)
  To: Zi Yan, Andrew Morton, Vlastimil Babka
  Cc: Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, linux-mm, linux-kernel, sunnanyong,
	wangkefeng.wang


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.

Qi




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning
  2026-08-19  7:55   ` Qi Xi
@ 2026-08-19 14:39     ` Zi Yan
  0 siblings, 0 replies; 4+ messages in thread
From: Zi Yan @ 2026-08-19 14:39 UTC (permalink / raw)
  To: Qi Xi, Andrew Morton, Vlastimil Babka
  Cc: Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, linux-mm, linux-kernel, sunnanyong,
	wangkefeng.wang

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-19 14:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 11:28 [PATCH] mm/page_isolation: fix UBSAN shift-out-of-bounds warning Qi Xi
2026-08-18 19:14 ` Zi Yan
2026-08-19  7:55   ` Qi Xi
2026-08-19 14:39     ` Zi Yan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.