linux-kernel - Re: [PATCH v3] mm/page_alloc: clear page->private in free_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <22431471-b569-4ade-9881-387debada00b@kernel.org>
Date: Mon, 9 Feb 2026 17:20:28 +0100
From: "David Hildenbrand (Arm)" <david@...nel.org>
To: Zi Yan <ziy@...dia.com>
Cc: Mikhail Gavrilov <mikhail.v.gavrilov@...il.com>, linux-mm@...ck.org,
 akpm@...ux-foundation.org, vbabka@...e.cz, surenb@...gle.com,
 mhocko@...e.com, jackmanb@...gle.com, hannes@...xchg.org, npiggin@...il.com,
 linux-kernel@...r.kernel.org, kasong@...cent.com, hughd@...gle.com,
 chrisl@...nel.org, ryncsn@...il.com, stable@...r.kernel.org,
 willy@...radead.org
Subject: Re: [PATCH v3] mm/page_alloc: clear page->private in
 free_pages_prepare()

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