linux-kernel - Re: [PATCH 1/2] huge_mm.h: is_huge_zero

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: <2aa3f478-9c87-4102-b83e-bf235372d834@redhat.com>
Date: Wed, 27 Aug 2025 16:34:36 +0200
From: David Hildenbrand <david@...hat.com>
To: Max Kellermann <max.kellermann@...os.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, lorenzo.stoakes@...cle.com,
 ziy@...dia.com, baolin.wang@...ux.alibaba.com, Liam.Howlett@...cle.com,
 npache@...hat.com, ryan.roberts@....com, dev.jain@....com,
 baohua@...nel.org, shikemeng@...weicloud.com, kasong@...cent.com,
 nphamcs@...il.com, bhe@...hat.com, chrisl@...nel.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] huge_mm.h: is_huge_zero_folio(NULL) should return
 false

On 27.08.25 15:06, Max Kellermann wrote:
> On Wed, Aug 27, 2025 at 1:56 PM David Hildenbrand <david@...hat.com> wrote:
>>> This patch isn't about the function parameter but about the global
>>> variable being NULL.
>>> (Don't mix up with my other patch.)
>>
>> Huh?
> 
> Oh, I see the misunderstanding. You are right here, yes my patch is
> indeed about passing NULL to is_huge_zero_folio(). My previous reply
> was confusing.
> 
> But you were replying to a sub-thread by Andrew who questioned whether
> a call is_huge_zero_folio() is legal at all before the huge zero folio
> has been created.

Sure it is, I mean on some systems it is never initialized.

> The question I asked Andrew, which you replied to, was not about
> passing NULL to is_huge_zero_folio(), but about whether the call to
> is_huge_zero_folio() is legal at all, no matter which parameter value.

Yes, see how get_huge_zero_folio() dynamically allocates it on demand to 
then update huge_zero_folio.

> 
> I agree with you that is_huge_zero_folio(NULL) should not be legal and
> makes no sense. It's a bug somewhere in the caller.
> But I saw that the current implementation effectively (randomly) hides
> a bug in another part of the kernel (= Ceph), one that I'd rather like
> to be visible (and get fixed). And that is the point of my patch, and
> is the reason why I chose to have the additional debug-only code
> inside an #ifdef - I don't want normal users to pay the price for a
> debugging feature, no matter how small the price.
> 
> David, what is your opinion here?
> 
> - leave it as it is; is_huge_zero_folio(NULL) may randomly return true
> or false (= reject this patch)
> - add the huge_zero_folio==NULL check, but guarded with #ifdef DEBUG
> (= this patch)
> - add the huge_zero_folio==NULL check unconditionally (suggested by Zi Yan)

VM_WARN_ON_ONCE(!folio);

Should allow for catching such bugs without causing any runtime overhead 
on non-debug kernels.


-- 
Cheers

David / dhildenb