mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
       [not found] <209207FE-D3A9-4BE2-8DA7-9BE38A19F387@nvidia.com>
@ 2026-02-07 17:36 ` Mikhail Gavrilov
  2026-02-07 22:02   ` David Hildenbrand (Arm)
  2026-02-09 19:46   ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 23+ messages in thread
From: Mikhail Gavrilov @ 2026-02-07 17:36 UTC (permalink / raw)
  To: linux-mm
  Cc: akpm, vbabka, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, david,
	willy, Mikhail Gavrilov

Several subsystems (slub, shmem, ttm, etc.) use page->private but don't
clear it before freeing pages. When these pages are later allocated as
high-order pages and split via split_page(), tail pages retain stale
page->private values.

This causes a use-after-free in the swap subsystem. The swap code uses
page->private to track swap count continuations, assuming freshly
allocated pages have page->private == 0. When stale values are present,
swap_count_continued() incorrectly assumes the continuation list is valid
and iterates over uninitialized page->lru containing LIST_POISON values,
causing a crash:

  KASAN: maybe wild-memory-access in range [0xdead000000000100-0xdead000000000107]
  RIP: 0010:__do_sys_swapoff+0x1151/0x1860

Fix this by clearing page->private in free_pages_prepare(), ensuring all
freed pages have clean state regardless of previous use.

Fixes: 3b8000ae185c ("mm/vmalloc: huge vmalloc backing pages should be split rather than compound")
Cc: stable@vger.kernel.org
Suggested-by: Zi Yan <ziy@nvidia.com>
Acked-by: Zi Yan <ziy@nvidia.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
 mm/page_alloc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cbf758e27aa2..24ac34199f95 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1430,6 +1430,7 @@ __always_inline bool free_pages_prepare(struct page *page,
 
 	page_cpupid_reset_last(page);
 	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+	page->private = 0;
 	reset_page_owner(page, order);
 	page_table_check_free(page, order);
 	pgalloc_tag_sub(page, 1 << order);
-- 
2.53.0


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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 17:36 ` [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare() Mikhail Gavrilov
@ 2026-02-07 22:02   ` David Hildenbrand (Arm)
  2026-02-07 22:08     ` David Hildenbrand (Arm)
  2026-02-07 23:00     ` Zi Yan
  2026-02-09 19:46   ` David Hildenbrand (Arm)
  1 sibling, 2 replies; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-07 22:02 UTC (permalink / raw)
  To: Mikhail Gavrilov, linux-mm
  Cc: akpm, vbabka, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, willy

On 2/7/26 18:36, Mikhail Gavrilov wrote:

Thanks!

> Several subsystems (slub, shmem, ttm, etc.) use page->private but don't
> clear it before freeing pages. When these pages are later allocated as
> high-order pages and split via split_page(), tail pages retain stale
> page->private values.
> 
> This causes a use-after-free in the swap subsystem. The swap code uses
> page->private to track swap count continuations, assuming freshly
> allocated pages have page->private == 0. When stale values are present,
> swap_count_continued() incorrectly assumes the continuation list is valid
> and iterates over uninitialized page->lru containing LIST_POISON values,
> causing a crash:
> 
>    KASAN: maybe wild-memory-access in range [0xdead000000000100-0xdead000000000107]
>    RIP: 0010:__do_sys_swapoff+0x1151/0x1860
> 
> Fix this by clearing page->private in free_pages_prepare(), ensuring all
> freed pages have clean state regardless of previous use.

I could have sworn we discussed something like that already in the past.

I recall that freeing pages with page->private set was allowed. Although
I once wondered whether we should actually change that.

> 
> Fixes: 3b8000ae185c ("mm/vmalloc: huge vmalloc backing pages should be split rather than compound")
> Cc: stable@vger.kernel.org
> Suggested-by: Zi Yan <ziy@nvidia.com>
> Acked-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---

Next time, please don't send patches as reply to another thread; that
way it can easily get lost in a bigger thread.

You want to get peoples attention :)

>   mm/page_alloc.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cbf758e27aa2..24ac34199f95 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1430,6 +1430,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>   
>   	page_cpupid_reset_last(page);
>   	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> +	page->private = 0;

Should we be using set_page_private()? It's a bit inconsistent :)

I wonder, if it's really just the split_page() problem, why not
handle it there, where we already iterate over all ("tail") pages?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cbf758e27aa2..cbbcfdf3ed26 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3122,8 +3122,10 @@ void split_page(struct page *page, unsigned int order)
         VM_BUG_ON_PAGE(PageCompound(page), page);
         VM_BUG_ON_PAGE(!page_count(page), page);
  
-       for (i = 1; i < (1 << order); i++)
+       for (i = 1; i < (1 << order); i++) {
                 set_page_refcounted(page + i);
+               set_page_private(page, 0);
+       }
         split_page_owner(page, order, 0);
         pgalloc_tag_split(page_folio(page), order, 0);
         split_page_memcg(page, order);


But then I thought about "what does actually happen during an folio split".

We had a check in __split_folio_to_order() that got removed in 4265d67e405a, for some
undocumented reason (and the patch got merged with 0 tags :( ). I assume because with zone-device
there was a way to now got ->private properly set. But we removed the safety check for
all other folios.

-               /*
-                * page->private should not be set in tail pages. Fix up and warn once
-                * if private is unexpectedly set.
-                */
-               if (unlikely(new_folio->private)) {
-                       VM_WARN_ON_ONCE_PAGE(true, new_head);
-                       new_folio->private = NULL;
-               }


I would have thought that we could have triggered that check easily before. Why didn't we?

Who would have cleared the private field of tail pages?

@Zi Yan, any idea why the folio splitting code wouldn't have revealed a similar problem?

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 22:02   ` David Hildenbrand (Arm)
@ 2026-02-07 22:08     ` David Hildenbrand (Arm)
  2026-02-09 11:17       ` Vlastimil Babka
  2026-02-07 23:00     ` Zi Yan
  1 sibling, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-07 22:08 UTC (permalink / raw)
  To: Mikhail Gavrilov, linux-mm
  Cc: akpm, vbabka, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, willy

> 
> -               /*
> -                * page->private should not be set in tail pages. Fix up 
> and warn once
> -                * if private is unexpectedly set.
> -                */
> -               if (unlikely(new_folio->private)) {
> -                       VM_WARN_ON_ONCE_PAGE(true, new_head);
> -                       new_folio->private = NULL;
> -               }

BTW, I wonder whether we should bring that check back for non-device folios.

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 22:02   ` David Hildenbrand (Arm)
  2026-02-07 22:08     ` David Hildenbrand (Arm)
@ 2026-02-07 23:00     ` Zi Yan
  2026-02-09 16:16       ` David Hildenbrand (Arm)
  1 sibling, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-07 23:00 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 7 Feb 2026, at 17:02, David Hildenbrand (Arm) wrote:

> On 2/7/26 18:36, Mikhail Gavrilov wrote:
>
> Thanks!
>
>> Several subsystems (slub, shmem, ttm, etc.) use page->private but don't
>> clear it before freeing pages. When these pages are later allocated as
>> high-order pages and split via split_page(), tail pages retain stale
>> page->private values.
>>
>> This causes a use-after-free in the swap subsystem. The swap code uses
>> page->private to track swap count continuations, assuming freshly
>> allocated pages have page->private == 0. When stale values are present,
>> swap_count_continued() incorrectly assumes the continuation list is valid
>> and iterates over uninitialized page->lru containing LIST_POISON values,
>> causing a crash:
>>
>>    KASAN: maybe wild-memory-access in range [0xdead000000000100-0xdead000000000107]
>>    RIP: 0010:__do_sys_swapoff+0x1151/0x1860
>>
>> Fix this by clearing page->private in free_pages_prepare(), ensuring all
>> freed pages have clean state regardless of previous use.
>
> I could have sworn we discussed something like that already in the past.

This[1] is my discussion on this topic and I managed to convince people we should
keep ->private zero on any pages.

[1] https://lore.kernel.org/all/20250925085006.23684-1-zhongjinji@honor.com/
>
> I recall that freeing pages with page->private set was allowed. Although
> I once wondered whether we should actually change that.

But if that is allowed, we can end up with tail page's private non zero,
because that free page can merge with a lower PFN buddy and its ->private
is not reset. See __free_one_page().

>
>>
>> Fixes: 3b8000ae185c ("mm/vmalloc: huge vmalloc backing pages should be split rather than compound")
>> Cc: stable@vger.kernel.org
>> Suggested-by: Zi Yan <ziy@nvidia.com>
>> Acked-by: Zi Yan <ziy@nvidia.com>
>> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
>> ---
>
> Next time, please don't send patches as reply to another thread; that
> way it can easily get lost in a bigger thread.
>
> You want to get peoples attention :)
>
>>   mm/page_alloc.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index cbf758e27aa2..24ac34199f95 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1430,6 +1430,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>    	page_cpupid_reset_last(page);
>>   	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>> +	page->private = 0;
>
> Should we be using set_page_private()? It's a bit inconsistent :)
>
> I wonder, if it's really just the split_page() problem, why not
> handle it there, where we already iterate over all ("tail") pages?
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index cbf758e27aa2..cbbcfdf3ed26 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3122,8 +3122,10 @@ void split_page(struct page *page, unsigned int order)
>         VM_BUG_ON_PAGE(PageCompound(page), page);
>         VM_BUG_ON_PAGE(!page_count(page), page);
>  -       for (i = 1; i < (1 << order); i++)
> +       for (i = 1; i < (1 << order); i++) {
>                 set_page_refcounted(page + i);
> +               set_page_private(page, 0);
> +       }
>         split_page_owner(page, order, 0);
>         pgalloc_tag_split(page_folio(page), order, 0);
>         split_page_memcg(page, order);
>
>
> But then I thought about "what does actually happen during an folio split".
>
> We had a check in __split_folio_to_order() that got removed in 4265d67e405a, for some
> undocumented reason (and the patch got merged with 0 tags :( ). I assume because with zone-device
> there was a way to now got ->private properly set. But we removed the safety check for

It was the end of last year and the review traffic was a lot. No one had time to look at
it.

> all other folios.
>
> -               /*
> -                * page->private should not be set in tail pages. Fix up and warn once
> -                * if private is unexpectedly set.
> -                */
> -               if (unlikely(new_folio->private)) {
> -                       VM_WARN_ON_ONCE_PAGE(true, new_head);
> -                       new_folio->private = NULL;
> -               }
>
>
> I would have thought that we could have triggered that check easily before. Why didn't we?
>
> Who would have cleared the private field of tail pages?
>
> @Zi Yan, any idea why the folio splitting code wouldn't have revealed a similar problem?
>

For the issue reported by Mikhail[2], the page comes from vmalloc(), so it will not be split.
For other cases, a page/folio needs to be compound to be splittable and prep_compound_tail()
sets all tail page's private to 0. So that check is not that useful.

And the issue we are handling here is non compound high order page allocation. No one is
clearing ->private for all pages right now.

OK, I think we want to decide whether it is OK to have a page with set ->private at
page free time. If no, we can get this patch in and add a VM_WARN_ON_ONCE(page->private)
to catch all violators. If yes, we can use Mikhail's original patch, zeroing ->private
in split_page() and add a comment on ->private:

1. for compound page allocation, prep_compound_tail() is responsible for resetting ->private;
2. for non compound high order page allocation, split_page() is responsible for resetting ->private.


[2] https://lore.kernel.org/linux-mm/CABXGCsNqk6pOkocJ0ctcHssCvke2kqhzoR2BGf_Hh1hWPZATuA@mail.gmail.com/



--
Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 22:08     ` David Hildenbrand (Arm)
@ 2026-02-09 11:17       ` Vlastimil Babka
  2026-02-09 15:46         ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: Vlastimil Babka @ 2026-02-09 11:17 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Mikhail Gavrilov, linux-mm
  Cc: akpm, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, willy

On 2/7/26 23:08, David Hildenbrand (Arm) wrote:
>> 
>> -               /*
>> -                * page->private should not be set in tail pages. Fix up 
>> and warn once
>> -                * if private is unexpectedly set.
>> -                */
>> -               if (unlikely(new_folio->private)) {
>> -                       VM_WARN_ON_ONCE_PAGE(true, new_head);
>> -                       new_folio->private = NULL;
>> -               }
> 
> BTW, I wonder whether we should bring that check back for non-device folios.

If the rule is now that when upon freeing in free_pages_prepare() we clear
private in the head page and not tail pages (where we expect the owner of
the page to do it), maybe that check for tail pages should be done in the
is_check_pages_enabled() part of free_pages_prepare().

Or should the check be also in the split path because somebody can set a
tail private between allocation and split? (and not just inherit it from a
previous allocation that didn't clear it?).

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 11:17       ` Vlastimil Babka
@ 2026-02-09 15:46         ` David Hildenbrand (Arm)
  2026-02-09 16:00           ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 15:46 UTC (permalink / raw)
  To: Vlastimil Babka, Mikhail Gavrilov, linux-mm
  Cc: akpm, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, willy

On 2/9/26 12:17, Vlastimil Babka wrote:
> On 2/7/26 23:08, David Hildenbrand (Arm) wrote:
>>>
>>> -               /*
>>> -                * page->private should not be set in tail pages. Fix up
>>> and warn once
>>> -                * if private is unexpectedly set.
>>> -                */
>>> -               if (unlikely(new_folio->private)) {
>>> -                       VM_WARN_ON_ONCE_PAGE(true, new_head);
>>> -                       new_folio->private = NULL;
>>> -               }
>>
>> BTW, I wonder whether we should bring that check back for non-device folios.
> 
> If the rule is now that when upon freeing in free_pages_prepare() we clear
> private in the head page and not tail pages (where we expect the owner of
> the page to do it), maybe that check for tail pages should be done in the
> is_check_pages_enabled() part of free_pages_prepare().
> 
> Or should the check be also in the split path because somebody can set a
> tail private between allocation and split? (and not just inherit it from a
> previous allocation that didn't clear it?).

We ran into that check in the past, when folio->X overlayed 
page->private in a tail, and would actually have to be zeroed out.

So it should be part of this splitting code I think.

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 15:46         ` David Hildenbrand (Arm)
@ 2026-02-09 16:00           ` Zi Yan
  2026-02-09 16:03             ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-09 16:00 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Vlastimil Babka, Mikhail Gavrilov, linux-mm, akpm, surenb,
	mhocko, jackmanb, hannes, npiggin, linux-kernel, kasong, hughd,
	chrisl, ryncsn, stable, willy

On 9 Feb 2026, at 10:46, David Hildenbrand (Arm) wrote:

> On 2/9/26 12:17, Vlastimil Babka wrote:
>> On 2/7/26 23:08, David Hildenbrand (Arm) wrote:
>>>>
>>>> -               /*
>>>> -                * page->private should not be set in tail pages. Fix up
>>>> and warn once
>>>> -                * if private is unexpectedly set.
>>>> -                */
>>>> -               if (unlikely(new_folio->private)) {
>>>> -                       VM_WARN_ON_ONCE_PAGE(true, new_head);
>>>> -                       new_folio->private = NULL;
>>>> -               }
>>>
>>> BTW, I wonder whether we should bring that check back for non-device folios.
>>
>> If the rule is now that when upon freeing in free_pages_prepare() we clear
>> private in the head page and not tail pages (where we expect the owner of
>> the page to do it), maybe that check for tail pages should be done in the
>> is_check_pages_enabled() part of free_pages_prepare().
>>
>> Or should the check be also in the split path because somebody can set a
>> tail private between allocation and split? (and not just inherit it from a
>> previous allocation that didn't clear it?).
>
> We ran into that check in the past, when folio->X overlayed page->private in a tail, and would actually have to be zeroed out.

Currently, _mm_id (_mm_ids) overlaps with page->private. At split time,
it should be MM_ID_DUMMY (0), so page->private should be 0 all time.

>
> So it should be part of this splitting code I think.

It is still better to have the check and fix in place. Why do we want to
skip device private folio?


Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:00           ` Zi Yan
@ 2026-02-09 16:03             ` David Hildenbrand (Arm)
  2026-02-09 16:05               ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 16:03 UTC (permalink / raw)
  To: Zi Yan
  Cc: Vlastimil Babka, Mikhail Gavrilov, linux-mm, akpm, surenb,
	mhocko, jackmanb, hannes, npiggin, linux-kernel, kasong, hughd,
	chrisl, ryncsn, stable, willy

On 2/9/26 17:00, Zi Yan wrote:
> On 9 Feb 2026, at 10:46, David Hildenbrand (Arm) wrote:
> 
>> On 2/9/26 12:17, Vlastimil Babka wrote:
>>>
>>> If the rule is now that when upon freeing in free_pages_prepare() we clear
>>> private in the head page and not tail pages (where we expect the owner of
>>> the page to do it), maybe that check for tail pages should be done in the
>>> is_check_pages_enabled() part of free_pages_prepare().
>>>
>>> Or should the check be also in the split path because somebody can set a
>>> tail private between allocation and split? (and not just inherit it from a
>>> previous allocation that didn't clear it?).
>>
>> We ran into that check in the past, when folio->X overlayed page->private in a tail, and would actually have to be zeroed out.
> 
> Currently, _mm_id (_mm_ids) overlaps with page->private. At split time,
> it should be MM_ID_DUMMY (0), so page->private should be 0 all time.

Yes, it's designed like that; because that check here caught it during 
development :)

> 
>>
>> So it should be part of this splitting code I think.
> 
> It is still better to have the check and fix in place. Why do we want to
> skip device private folio?

I don't understand the question, can you elaborate?

I asked Balbir why the check was dropped in the first place.

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:03             ` David Hildenbrand (Arm)
@ 2026-02-09 16:05               ` Zi Yan
  2026-02-09 16:06                 ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-09 16:05 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Vlastimil Babka, Mikhail Gavrilov, linux-mm, akpm, surenb,
	mhocko, jackmanb, hannes, npiggin, linux-kernel, kasong, hughd,
	chrisl, ryncsn, stable, willy

On 9 Feb 2026, at 11:03, David Hildenbrand (Arm) wrote:

> On 2/9/26 17:00, Zi Yan wrote:
>> On 9 Feb 2026, at 10:46, David Hildenbrand (Arm) wrote:
>>
>>> On 2/9/26 12:17, Vlastimil Babka wrote:
>>>>
>>>> If the rule is now that when upon freeing in free_pages_prepare() we clear
>>>> private in the head page and not tail pages (where we expect the owner of
>>>> the page to do it), maybe that check for tail pages should be done in the
>>>> is_check_pages_enabled() part of free_pages_prepare().
>>>>
>>>> Or should the check be also in the split path because somebody can set a
>>>> tail private between allocation and split? (and not just inherit it from a
>>>> previous allocation that didn't clear it?).
>>>
>>> We ran into that check in the past, when folio->X overlayed page->private in a tail, and would actually have to be zeroed out.
>>
>> Currently, _mm_id (_mm_ids) overlaps with page->private. At split time,
>> it should be MM_ID_DUMMY (0), so page->private should be 0 all time.
>
> Yes, it's designed like that; because that check here caught it during development :)
>
>>
>>>
>>> So it should be part of this splitting code I think.
>>
>> It is still better to have the check and fix in place. Why do we want to
>> skip device private folio?
>
> I don't understand the question, can you elaborate?

You said,
“BTW, I wonder whether we should bring that check back for non-device folios.”

I thought you know why device folio needs to keep ->private not cleared during
split.


> I asked Balbir why the check was dropped in the first place.


Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:05               ` Zi Yan
@ 2026-02-09 16:06                 ` David Hildenbrand (Arm)
  2026-02-09 16:08                   ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 16:06 UTC (permalink / raw)
  To: Zi Yan
  Cc: Vlastimil Babka, Mikhail Gavrilov, linux-mm, akpm, surenb,
	mhocko, jackmanb, hannes, npiggin, linux-kernel, kasong, hughd,
	chrisl, ryncsn, stable, willy

On 2/9/26 17:05, Zi Yan wrote:
> On 9 Feb 2026, at 11:03, David Hildenbrand (Arm) wrote:
> 
>> On 2/9/26 17:00, Zi Yan wrote:
>>>
>>>
>>> Currently, _mm_id (_mm_ids) overlaps with page->private. At split time,
>>> it should be MM_ID_DUMMY (0), so page->private should be 0 all time.
>>
>> Yes, it's designed like that; because that check here caught it during development :)
>>
>>>
>>>
>>> It is still better to have the check and fix in place. Why do we want to
>>> skip device private folio?
>>
>> I don't understand the question, can you elaborate?
> 
> You said,
> “BTW, I wonder whether we should bring that check back for non-device folios.”
> 
> I thought you know why device folio needs to keep ->private not cleared during
> split.

Oh, I thought there was some overlay of ->private with zone-device 
special stuff. But I checked the structs and didn't spot it immediately. 
So I ended up asking Balbir as reply to his latest series that got merged.

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:06                 ` David Hildenbrand (Arm)
@ 2026-02-09 16:08                   ` Zi Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Zi Yan @ 2026-02-09 16:08 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Vlastimil Babka, Mikhail Gavrilov, linux-mm, akpm, surenb,
	mhocko, jackmanb, hannes, npiggin, linux-kernel, kasong, hughd,
	chrisl, ryncsn, stable, willy

On 9 Feb 2026, at 11:06, David Hildenbrand (Arm) wrote:

> On 2/9/26 17:05, Zi Yan wrote:
>> On 9 Feb 2026, at 11:03, David Hildenbrand (Arm) wrote:
>>
>>> On 2/9/26 17:00, Zi Yan wrote:
>>>>
>>>>
>>>> Currently, _mm_id (_mm_ids) overlaps with page->private. At split time,
>>>> it should be MM_ID_DUMMY (0), so page->private should be 0 all time.
>>>
>>> Yes, it's designed like that; because that check here caught it during development :)
>>>
>>>>
>>>>
>>>> It is still better to have the check and fix in place. Why do we want to
>>>> skip device private folio?
>>>
>>> I don't understand the question, can you elaborate?
>>
>> You said,
>> “BTW, I wonder whether we should bring that check back for non-device folios.”
>>
>> I thought you know why device folio needs to keep ->private not cleared during
>> split.
>
> Oh, I thought there was some overlay of ->private with zone-device special stuff. But I checked the structs and didn't spot it immediately. So I ended up asking Balbir as reply to his latest series that got merged.

Got it. We will bring it back once we hear back from Balbir. Thanks.

Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 23:00     ` Zi Yan
@ 2026-02-09 16:16       ` David Hildenbrand (Arm)
  2026-02-09 16:20         ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 16:16 UTC (permalink / raw)
  To: Zi Yan
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

>> I recall that freeing pages with page->private set was allowed. Although
>> I once wondered whether we should actually change that.
> 
> But if that is allowed, we can end up with tail page's private non zero,
> because that free page can merge with a lower PFN buddy and its ->private
> is not reset. See __free_one_page().

Right. Or someone could use page->private on tail pages and free non-zero ->private that way.

[...]

> 
> For the issue reported by Mikhail[2], the page comes from vmalloc(), so it will not be split.
> For other cases, a page/folio needs to be compound to be splittable and prep_compound_tail()
> sets all tail page's private to 0. So that check is not that useful.

Thanks.

> 
> And the issue we are handling here is non compound high order page allocation. No one is
> clearing ->private for all pages right now.

Right.

> 
> OK, I think we want to decide whether it is OK to have a page with set ->private at
> page free time.

Right. And whether it is okay to have any tail->private be non-zero.

> If no, we can get this patch in and add a VM_WARN_ON_ONCE(page->private)
> to catch all violators. If yes, we can use Mikhail's original patch, zeroing ->private
> in split_page() and add a comment on ->private:
> 
> 1. for compound page allocation, prep_compound_tail() is responsible for resetting ->private;
> 2. for non compound high order page allocation, split_page() is responsible for resetting ->private.

Ideally, I guess, we would minimize the clearing of the ->private fields.

If we could guarantee that *any* pages in the buddy have ->private clear, maybe
prep_compound_tail() could stop clearing it (and check instead).

So similar to what Vlasta said, maybe we want to (not check but actually clear):


diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e4104973e22f..4960a36145fe 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct page *page,
                                 }
                         }
                         (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+                       set_page_private(page + i, 0);
                 }
         }
         if (folio_test_anon(folio)) {


And the try removing any other unnecessary clearing (like in prep_compound_tail()).


-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:16       ` David Hildenbrand (Arm)
@ 2026-02-09 16:20         ` David Hildenbrand (Arm)
  2026-02-09 16:33           ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 16:20 UTC (permalink / raw)
  To: Zi Yan
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 2/9/26 17:16, David Hildenbrand (Arm) wrote:
>>> I recall that freeing pages with page->private set was allowed. Although
>>> I once wondered whether we should actually change that.
>>
>> But if that is allowed, we can end up with tail page's private non zero,
>> because that free page can merge with a lower PFN buddy and its ->private
>> is not reset. See __free_one_page().
> 
> Right. Or someone could use page->private on tail pages and free non- 
> zero ->private that way.
> 
> [...]
> 
>>
>> For the issue reported by Mikhail[2], the page comes from vmalloc(), 
>> so it will not be split.
>> For other cases, a page/folio needs to be compound to be splittable 
>> and prep_compound_tail()
>> sets all tail page's private to 0. So that check is not that useful.
> 
> Thanks.
> 
>>
>> And the issue we are handling here is non compound high order page 
>> allocation. No one is
>> clearing ->private for all pages right now.
> 
> Right.
> 
>>
>> OK, I think we want to decide whether it is OK to have a page with set 
>> ->private at
>> page free time.
> 
> Right. And whether it is okay to have any tail->private be non-zero.
> 
>> If no, we can get this patch in and add a VM_WARN_ON_ONCE(page->private)
>> to catch all violators. If yes, we can use Mikhail's original patch, 
>> zeroing ->private
>> in split_page() and add a comment on ->private:
>>
>> 1. for compound page allocation, prep_compound_tail() is responsible 
>> for resetting ->private;
>> 2. for non compound high order page allocation, split_page() is 
>> responsible for resetting ->private.
> 
> Ideally, I guess, we would minimize the clearing of the ->private fields.
> 
> If we could guarantee that *any* pages in the buddy have ->private 
> clear, maybe
> prep_compound_tail() could stop clearing it (and check instead).
> 
> So similar to what Vlasta said, maybe we want to (not check but actually 
> clear):
> 
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e4104973e22f..4960a36145fe 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct 
> page *page,
>                                  }
>                          }
>                          (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> +                       set_page_private(page + i, 0);
>                  }
>          }

Thinking again, maybe it is indeed better to rework the code to not 
allow freeing pages with ->private on any page. Then, we only have to 
zero it out where we actually used it and could check here that all
->private is 0.

I guess that's a bit more work, and any temporary fix would likely just do.

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:20         ` David Hildenbrand (Arm)
@ 2026-02-09 16:33           ` Zi Yan
  2026-02-09 17:36             ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-09 16:33 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 9 Feb 2026, at 11:20, David Hildenbrand (Arm) wrote:

> On 2/9/26 17:16, David Hildenbrand (Arm) wrote:
>>>> I recall that freeing pages with page->private set was allowed. Although
>>>> I once wondered whether we should actually change that.
>>>
>>> But if that is allowed, we can end up with tail page's private non zero,
>>> because that free page can merge with a lower PFN buddy and its ->private
>>> is not reset. See __free_one_page().
>>
>> Right. Or someone could use page->private on tail pages and free non- zero ->private that way.
>>
>> [...]
>>
>>>
>>> For the issue reported by Mikhail[2], the page comes from vmalloc(), so it will not be split.
>>> For other cases, a page/folio needs to be compound to be splittable and prep_compound_tail()
>>> sets all tail page's private to 0. So that check is not that useful.
>>
>> Thanks.
>>
>>>
>>> And the issue we are handling here is non compound high order page allocation. No one is
>>> clearing ->private for all pages right now.
>>
>> Right.
>>
>>>
>>> OK, I think we want to decide whether it is OK to have a page with set ->private at
>>> page free time.
>>
>> Right. And whether it is okay to have any tail->private be non-zero.
>>
>>> If no, we can get this patch in and add a VM_WARN_ON_ONCE(page->private)
>>> to catch all violators. If yes, we can use Mikhail's original patch, zeroing ->private
>>> in split_page() and add a comment on ->private:
>>>
>>> 1. for compound page allocation, prep_compound_tail() is responsible for resetting ->private;
>>> 2. for non compound high order page allocation, split_page() is responsible for resetting ->private.
>>
>> Ideally, I guess, we would minimize the clearing of the ->private fields.
>>
>> If we could guarantee that *any* pages in the buddy have ->private clear, maybe
>> prep_compound_tail() could stop clearing it (and check instead).
>>
>> So similar to what Vlasta said, maybe we want to (not check but actually clear):
>>
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index e4104973e22f..4960a36145fe 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>                                  }
>>                          }
>>                          (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>> +                       set_page_private(page + i, 0);
>>                  }
>>          }
>
> Thinking again, maybe it is indeed better to rework the code to not allow freeing pages with ->private on any page. Then, we only have to zero it out where we actually used it and could check here that all
> ->private is 0.
>
> I guess that's a bit more work, and any temporary fix would likely just do.

I agree. Silently fixing non zero ->private just moves the work/responsibility
from users to core mm. They could do better. :)

We can have a patch or multiple patches to fix users do not zero ->private
when freeing a page and add the patch below. The hassle would be that
catching all, especially non mm users might not be easy, but we could merge
the patch below (and obviously fixes) after next merge window is closed and
let rc tests tell us the remaining one. WDYT?


diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 24ac34199f95..0c5d117a251e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1411,6 +1411,7 @@ __always_inline bool free_pages_prepare(struct page *page,
 				}
 			}
 			(page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+			VM_WARN_ON_ONCE((page + i)->private);
 		}
 	}
 	if (folio_test_anon(folio)) {
@@ -1430,6 +1431,7 @@ __always_inline bool free_pages_prepare(struct page *page,

 	page_cpupid_reset_last(page);
 	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
+	VM_WARN_ON_ONCE(page->private);
 	page->private = 0;
 	reset_page_owner(page, order);
 	page_table_check_free(page, order);


Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 16:33           ` Zi Yan
@ 2026-02-09 17:36             ` David Hildenbrand (Arm)
  2026-02-09 17:44               ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 17:36 UTC (permalink / raw)
  To: Zi Yan
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 2/9/26 17:33, Zi Yan wrote:
> On 9 Feb 2026, at 11:20, David Hildenbrand (Arm) wrote:
> 
>> On 2/9/26 17:16, David Hildenbrand (Arm) wrote:
>>>
>>> Right. Or someone could use page->private on tail pages and free non- zero ->private that way.
>>>
>>> [...]
>>>
>>>
>>> Thanks.
>>>
>>>
>>> Right.
>>>
>>>
>>> Right. And whether it is okay to have any tail->private be non-zero.
>>>
>>>
>>> Ideally, I guess, we would minimize the clearing of the ->private fields.
>>>
>>> If we could guarantee that *any* pages in the buddy have ->private clear, maybe
>>> prep_compound_tail() could stop clearing it (and check instead).
>>>
>>> So similar to what Vlasta said, maybe we want to (not check but actually clear):
>>>
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index e4104973e22f..4960a36145fe 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>>                                   }
>>>                           }
>>>                           (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>> +                       set_page_private(page + i, 0);
>>>                   }
>>>           }
>>
>> Thinking again, maybe it is indeed better to rework the code to not allow freeing pages with ->private on any page. Then, we only have to zero it out where we actually used it and could check here that all
>> ->private is 0.
>>
>> I guess that's a bit more work, and any temporary fix would likely just do.
> 
> I agree. Silently fixing non zero ->private just moves the work/responsibility
> from users to core mm. They could do better. :)
> 
> We can have a patch or multiple patches to fix users do not zero ->private
> when freeing a page and add the patch below.

Do we know roughly which ones don't zero it out?

> The hassle would be that
> catching all, especially non mm users might not be easy, but we could merge
> the patch below (and obviously fixes) after next merge window is closed and
> let rc tests tell us the remaining one. WDYT?

LGTM, then we can look into stopping to zero for compound pages.

> 
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 24ac34199f95..0c5d117a251e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1411,6 +1411,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>   				}
>   			}
>   			(page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> +			VM_WARN_ON_ONCE((page + i)->private);
>   		}
>   	}
>   	if (folio_test_anon(folio)) {
> @@ -1430,6 +1431,7 @@ __always_inline bool free_pages_prepare(struct page *page,
> 
>   	page_cpupid_reset_last(page);
>   	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> +	VM_WARN_ON_ONCE(page->private);
>   	page->private = 0;
>   	reset_page_owner(page, order);
>   	page_table_check_free(page, order);
> 
> 
> Best Regards,
> Yan, Zi


-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 17:36             ` David Hildenbrand (Arm)
@ 2026-02-09 17:44               ` Zi Yan
  2026-02-09 19:39                 ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-09 17:44 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:

> On 2/9/26 17:33, Zi Yan wrote:
>> On 9 Feb 2026, at 11:20, David Hildenbrand (Arm) wrote:
>>
>>> On 2/9/26 17:16, David Hildenbrand (Arm) wrote:
>>>>
>>>> Right. Or someone could use page->private on tail pages and free non- zero ->private that way.
>>>>
>>>> [...]
>>>>
>>>>
>>>> Thanks.
>>>>
>>>>
>>>> Right.
>>>>
>>>>
>>>> Right. And whether it is okay to have any tail->private be non-zero.
>>>>
>>>>
>>>> Ideally, I guess, we would minimize the clearing of the ->private fields.
>>>>
>>>> If we could guarantee that *any* pages in the buddy have ->private clear, maybe
>>>> prep_compound_tail() could stop clearing it (and check instead).
>>>>
>>>> So similar to what Vlasta said, maybe we want to (not check but actually clear):
>>>>
>>>>
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index e4104973e22f..4960a36145fe 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -1410,6 +1410,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>>>                                   }
>>>>                           }
>>>>                           (page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>>>> +                       set_page_private(page + i, 0);
>>>>                   }
>>>>           }
>>>
>>> Thinking again, maybe it is indeed better to rework the code to not allow freeing pages with ->private on any page. Then, we only have to zero it out where we actually used it and could check here that all
>>> ->private is 0.
>>>
>>> I guess that's a bit more work, and any temporary fix would likely just do.
>>
>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>> from users to core mm. They could do better. :)
>>
>> We can have a patch or multiple patches to fix users do not zero ->private
>> when freeing a page and add the patch below.
>
> Do we know roughly which ones don't zero it out?

So far based on [1], I found:

1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
with private);
2. __free_slab() in mm/slub.c does not zero ->inuse, ->objects, ->frozen
(overlapping with private).

Mikhail found ttm_pool_unmap_and_free() in drivers/gpu/drm/ttm/ttm_pool.c
does not zero ->private, which stores page order.


[1] https://lore.kernel.org/all/CABXGCsNyt6DB=SX9JWD=-WK_BiHhbXaCPNV-GOM8GskKJVAn_A@mail.gmail.com/

>
>> The hassle would be that
>> catching all, especially non mm users might not be easy, but we could merge
>> the patch below (and obviously fixes) after next merge window is closed and
>> let rc tests tell us the remaining one. WDYT?
>
> LGTM, then we can look into stopping to zero for compound pages.
>
>>
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 24ac34199f95..0c5d117a251e 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1411,6 +1411,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>   				}
>>   			}
>>   			(page + i)->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>> +			VM_WARN_ON_ONCE((page + i)->private);
>>   		}
>>   	}
>>   	if (folio_test_anon(folio)) {
>> @@ -1430,6 +1431,7 @@ __always_inline bool free_pages_prepare(struct page *page,
>>
>>   	page_cpupid_reset_last(page);
>>   	page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
>> +	VM_WARN_ON_ONCE(page->private);
>>   	page->private = 0;
>>   	reset_page_owner(page, order);
>>   	page_table_check_free(page, order);
>>
>>
>> Best Regards,
>> Yan, Zi
>
>
> -- 
> Cheers,
>
> David


Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 17:44               ` Zi Yan
@ 2026-02-09 19:39                 ` David Hildenbrand (Arm)
  2026-02-09 19:42                   ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 19:39 UTC (permalink / raw)
  To: Zi Yan
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 2/9/26 18:44, Zi Yan wrote:
> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
> 
>> On 2/9/26 17:33, Zi Yan wrote:
>>>
>>>
>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>> from users to core mm. They could do better. :)
>>>
>>> We can have a patch or multiple patches to fix users do not zero ->private
>>> when freeing a page and add the patch below.
>>
>> Do we know roughly which ones don't zero it out?
> 
> So far based on [1], I found:
> 
> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
> with private);
> 2. __free_slab() in mm/slub.c does not zero ->inuse, ->objects, ->frozen
> (overlapping with private).
> 
> Mikhail found ttm_pool_unmap_and_free() in drivers/gpu/drm/ttm/ttm_pool.c
> does not zero ->private, which stores page order.
>

Looks doable then :) Should we take v3 as a quick fix to backport then?

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 19:39                 ` David Hildenbrand (Arm)
@ 2026-02-09 19:42                   ` Zi Yan
  2026-02-10  1:20                     ` Baolin Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-09 19:42 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:

> On 2/9/26 18:44, Zi Yan wrote:
>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>
>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>
>>>>
>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>> from users to core mm. They could do better. :)
>>>>
>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>> when freeing a page and add the patch below.
>>>
>>> Do we know roughly which ones don't zero it out?
>>
>> So far based on [1], I found:
>>
>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>> with private);
>> 2. __free_slab() in mm/slub.c does not zero ->inuse, ->objects, ->frozen
>> (overlapping with private).
>>
>> Mikhail found ttm_pool_unmap_and_free() in drivers/gpu/drm/ttm/ttm_pool.c
>> does not zero ->private, which stores page order.
>>
>
> Looks doable then :) Should we take v3 as a quick fix to backport then?

Sounds good to me.

Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-07 17:36 ` [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare() Mikhail Gavrilov
  2026-02-07 22:02   ` David Hildenbrand (Arm)
@ 2026-02-09 19:46   ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 23+ messages in thread
From: David Hildenbrand (Arm) @ 2026-02-09 19:46 UTC (permalink / raw)
  To: Mikhail Gavrilov, linux-mm
  Cc: akpm, vbabka, surenb, mhocko, jackmanb, hannes, ziy, npiggin,
	linux-kernel, kasong, hughd, chrisl, ryncsn, stable, willy

On 2/7/26 18:36, Mikhail Gavrilov wrote:
> Several subsystems (slub, shmem, ttm, etc.) use page->private but don't
> clear it before freeing pages. When these pages are later allocated as
> high-order pages and split via split_page(), tail pages retain stale
> page->private values.
> 
> This causes a use-after-free in the swap subsystem. The swap code uses
> page->private to track swap count continuations, assuming freshly
> allocated pages have page->private == 0. When stale values are present,
> swap_count_continued() incorrectly assumes the continuation list is valid
> and iterates over uninitialized page->lru containing LIST_POISON values,
> causing a crash:
> 
>    KASAN: maybe wild-memory-access in range [0xdead000000000100-0xdead000000000107]
>    RIP: 0010:__do_sys_swapoff+0x1151/0x1860
> 
> Fix this by clearing page->private in free_pages_prepare(), ensuring all
> freed pages have clean state regardless of previous use.
> 
> Fixes: 3b8000ae185c ("mm/vmalloc: huge vmalloc backing pages should be split rather than compound")
> Cc: stable@vger.kernel.org
> Suggested-by: Zi Yan <ziy@nvidia.com>
> Acked-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---

Okay, let's do this as a fix, and cleanup the page->private handling 
separately.

Thanks!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-09 19:42                   ` Zi Yan
@ 2026-02-10  1:20                     ` Baolin Wang
  2026-02-10  2:12                       ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: Baolin Wang @ 2026-02-10  1:20 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand (Arm)
  Cc: Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy



On 2/10/26 3:42 AM, Zi Yan wrote:
> On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:
> 
>> On 2/9/26 18:44, Zi Yan wrote:
>>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>>
>>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>>
>>>>>
>>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>>> from users to core mm. They could do better. :)
>>>>>
>>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>>> when freeing a page and add the patch below.
>>>>
>>>> Do we know roughly which ones don't zero it out?
>>>
>>> So far based on [1], I found:
>>>
>>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>>> with private);

After Kairui’s series [1], the shmem part looks good to me. As we no 
longer skip the swapcache now, we shouldn’t clear the ->swap.val of a 
swapcache folio if failed to swap-in.

[1]https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#mcba8a32e1021dc28ce1e824c9d042dca316a30d7

>>> 2. __free_slab() in mm/slub.c does not zero ->inuse, ->objects, ->frozen
>>> (overlapping with private).
>>>
>>> Mikhail found ttm_pool_unmap_and_free() in drivers/gpu/drm/ttm/ttm_pool.c
>>> does not zero ->private, which stores page order.


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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-10  1:20                     ` Baolin Wang
@ 2026-02-10  2:12                       ` Zi Yan
  2026-02-10  2:25                         ` Baolin Wang
  0 siblings, 1 reply; 23+ messages in thread
From: Zi Yan @ 2026-02-10  2:12 UTC (permalink / raw)
  To: Baolin Wang
  Cc: David Hildenbrand (Arm),
	Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 9 Feb 2026, at 20:20, Baolin Wang wrote:

> On 2/10/26 3:42 AM, Zi Yan wrote:
>> On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:
>>
>>> On 2/9/26 18:44, Zi Yan wrote:
>>>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>>>
>>>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>>>
>>>>>>
>>>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>>>> from users to core mm. They could do better. :)
>>>>>>
>>>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>>>> when freeing a page and add the patch below.
>>>>>
>>>>> Do we know roughly which ones don't zero it out?
>>>>
>>>> So far based on [1], I found:
>>>>
>>>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>>>> with private);
>
> After Kairui’s series [1], the shmem part looks good to me. As we no longer skip the swapcache now, we shouldn’t clear the ->swap.val of a swapcache folio if failed to swap-in.

What do you mean by "after Kairui's series[1]"? Can you elaborate a little bit more?

For the diff below, does the "folio_put(folio)" have different outcomes based on
skip_swapcache? Only if skip_swapcache is true, "folio_put(folio)" frees the folio?

Thanks.

diff --git a/mm/shmem.c b/mm/shmem.c
index ec6c01378e9d..546e193ef993 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2437,8 +2437,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
 failed_nolock:
        if (skip_swapcache)
                swapcache_clear(si, folio->swap, folio_nr_pages(folio));
-       if (folio)
+       if (folio) {
+               folio->swap.val = 0;
                folio_put(folio);
+       }
        put_swap_device(si);

        return error;

>
> [1]https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#mcba8a32e1021dc28ce1e824c9d042dca316a30d7
>
>>>> 2. __free_slab() in mm/slub.c does not zero ->inuse, ->objects, ->frozen
>>>> (overlapping with private).
>>>>
>>>> Mikhail found ttm_pool_unmap_and_free() in drivers/gpu/drm/ttm/ttm_pool.c
>>>> does not zero ->private, which stores page order.


--
Best Regards,
Yan, Zi

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-10  2:12                       ` Zi Yan
@ 2026-02-10  2:25                         ` Baolin Wang
  2026-02-10  2:32                           ` Zi Yan
  0 siblings, 1 reply; 23+ messages in thread
From: Baolin Wang @ 2026-02-10  2:25 UTC (permalink / raw)
  To: Zi Yan
  Cc: David Hildenbrand (Arm),
	Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy



On 2/10/26 10:12 AM, Zi Yan wrote:
> On 9 Feb 2026, at 20:20, Baolin Wang wrote:
> 
>> On 2/10/26 3:42 AM, Zi Yan wrote:
>>> On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:
>>>
>>>> On 2/9/26 18:44, Zi Yan wrote:
>>>>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>>>>
>>>>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>>>>
>>>>>>>
>>>>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>>>>> from users to core mm. They could do better. :)
>>>>>>>
>>>>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>>>>> when freeing a page and add the patch below.
>>>>>>
>>>>>> Do we know roughly which ones don't zero it out?
>>>>>
>>>>> So far based on [1], I found:
>>>>>
>>>>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>>>>> with private);
>>
>> After Kairui’s series [1], the shmem part looks good to me. As we no longer skip the swapcache now, we shouldn’t clear the ->swap.val of a swapcache folio if failed to swap-in.
> 
> What do you mean by "after Kairui's series[1]"? Can you elaborate a little bit more?

Sure. This patch [2] in Kairui's series will never skip the swapcache, 
which means the shmem folio we’re trying to swap-in must be in the 
swapcache.

[2] 
https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#me242d9f77d2caa126124afd5a7731113e8f0346e

> For the diff below, does the "folio_put(folio)" have different outcomes based on
> skip_swapcache? Only if skip_swapcache is true, "folio_put(folio)" frees the folio?

Please check the latest mm-stable branch. The skip_swapcache related 
logic has been removed by Kairui’s series [1].

> diff --git a/mm/shmem.c b/mm/shmem.c
> index ec6c01378e9d..546e193ef993 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2437,8 +2437,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
>   failed_nolock:
>          if (skip_swapcache)
>                  swapcache_clear(si, folio->swap, folio_nr_pages(folio));
> -       if (folio)
> +       if (folio) {
> +               folio->swap.val = 0;
>                  folio_put(folio);
> +       }
>          put_swap_device(si);
> 
>          return error;

Without Kairui's series, this change is incorrect. Yes, only if 
skip_swapcache is true, the "folio_put(folio)" frees the folio. 
Otherwise the folio is in the swapcache, and we will not free it.

>> [1]https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#mcba8a32e1021dc28ce1e824c9d042dca316a30d7

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

* Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare()
  2026-02-10  2:25                         ` Baolin Wang
@ 2026-02-10  2:32                           ` Zi Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Zi Yan @ 2026-02-10  2:32 UTC (permalink / raw)
  To: Baolin Wang
  Cc: David Hildenbrand (Arm),
	Mikhail Gavrilov, linux-mm, akpm, vbabka, surenb, mhocko,
	jackmanb, hannes, npiggin, linux-kernel, kasong, hughd, chrisl,
	ryncsn, stable, willy

On 9 Feb 2026, at 21:25, Baolin Wang wrote:

> On 2/10/26 10:12 AM, Zi Yan wrote:
>> On 9 Feb 2026, at 20:20, Baolin Wang wrote:
>>
>>> On 2/10/26 3:42 AM, Zi Yan wrote:
>>>> On 9 Feb 2026, at 14:39, David Hildenbrand (Arm) wrote:
>>>>
>>>>> On 2/9/26 18:44, Zi Yan wrote:
>>>>>> On 9 Feb 2026, at 12:36, David Hildenbrand (Arm) wrote:
>>>>>>
>>>>>>> On 2/9/26 17:33, Zi Yan wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> I agree. Silently fixing non zero ->private just moves the work/responsibility
>>>>>>>> from users to core mm. They could do better. :)
>>>>>>>>
>>>>>>>> We can have a patch or multiple patches to fix users do not zero ->private
>>>>>>>> when freeing a page and add the patch below.
>>>>>>>
>>>>>>> Do we know roughly which ones don't zero it out?
>>>>>>
>>>>>> So far based on [1], I found:
>>>>>>
>>>>>> 1. shmem_swapin_folio() in mm/shmem.c does not zero ->swap.val (overlapping
>>>>>> with private);
>>>
>>> After Kairui’s series [1], the shmem part looks good to me. As we no longer skip the swapcache now, we shouldn’t clear the ->swap.val of a swapcache folio if failed to swap-in.
>>
>> What do you mean by "after Kairui's series[1]"? Can you elaborate a little bit more?
>
> Sure. This patch [2] in Kairui's series will never skip the swapcache, which means the shmem folio we’re trying to swap-in must be in the swapcache.
>
> [2] https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#me242d9f77d2caa126124afd5a7731113e8f0346e
>
>> For the diff below, does the "folio_put(folio)" have different outcomes based on
>> skip_swapcache? Only if skip_swapcache is true, "folio_put(folio)" frees the folio?
>
> Please check the latest mm-stable branch. The skip_swapcache related logic has been removed by Kairui’s series [1].
>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index ec6c01378e9d..546e193ef993 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -2437,8 +2437,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
>>   failed_nolock:
>>          if (skip_swapcache)
>>                  swapcache_clear(si, folio->swap, folio_nr_pages(folio));
>> -       if (folio)
>> +       if (folio) {
>> +               folio->swap.val = 0;
>>                  folio_put(folio);
>> +       }
>>          put_swap_device(si);
>>
>>          return error;
>
> Without Kairui's series, this change is incorrect. Yes, only if skip_swapcache is true, the "folio_put(folio)" frees the folio. Otherwise the folio is in the swapcache, and we will not free it.

Got it. Thanks. I just realized that the above diff is on top of v6.19-rc7.
The fix to mm-new/mm-stable for shmem should be:

diff --git a/mm/shmem.c b/mm/shmem.c
index eaaeca8f6c39..a52eca656ade 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2447,8 +2447,10 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
 	if (folio)
 		folio_unlock(folio);
 failed_nolock:
-	if (folio)
+	if (folio) {
+		folio->swap.val = 0;
 		folio_put(folio);
+	}
 	put_swap_device(si);

 	return error;

Thank you for the explanation.

>
>>> [1]https://lore.kernel.org/all/20251219195751.61328-1-ryncsn@gmail.com/T/#mcba8a32e1021dc28ce1e824c9d042dca316a30d7


--
Best Regards,
Yan, Zi

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

end of thread, other threads:[~2026-02-10  2:32 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <209207FE-D3A9-4BE2-8DA7-9BE38A19F387@nvidia.com>
2026-02-07 17:36 ` [PATCH v3] mm/page_alloc: clear page->private in free_pages_prepare() Mikhail Gavrilov
2026-02-07 22:02   ` David Hildenbrand (Arm)
2026-02-07 22:08     ` David Hildenbrand (Arm)
2026-02-09 11:17       ` Vlastimil Babka
2026-02-09 15:46         ` David Hildenbrand (Arm)
2026-02-09 16:00           ` Zi Yan
2026-02-09 16:03             ` David Hildenbrand (Arm)
2026-02-09 16:05               ` Zi Yan
2026-02-09 16:06                 ` David Hildenbrand (Arm)
2026-02-09 16:08                   ` Zi Yan
2026-02-07 23:00     ` Zi Yan
2026-02-09 16:16       ` David Hildenbrand (Arm)
2026-02-09 16:20         ` David Hildenbrand (Arm)
2026-02-09 16:33           ` Zi Yan
2026-02-09 17:36             ` David Hildenbrand (Arm)
2026-02-09 17:44               ` Zi Yan
2026-02-09 19:39                 ` David Hildenbrand (Arm)
2026-02-09 19:42                   ` Zi Yan
2026-02-10  1:20                     ` Baolin Wang
2026-02-10  2:12                       ` Zi Yan
2026-02-10  2:25                         ` Baolin Wang
2026-02-10  2:32                           ` Zi Yan
2026-02-09 19:46   ` David Hildenbrand (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome