linux-kernel - Re: [PATCH v3 1/2] x86/mm: Handle alloc failure in phys_*

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: <f6834f95-49d0-4948-937c-e6eb7704ee35@neon.tech>
Date: Wed, 11 Jun 2025 20:26:09 +0100
From: Em Sharnoff <sharnoff@...n.tech>
To: Dave Hansen <dave.hansen@...el.com>, linux-kernel@...r.kernel.org,
 x86@...nel.org, linux-mm@...ck.org
Cc: Ingo Molnar <mingo@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
 Andy Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
 Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>,
 "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
 Oleg Vasilev <oleg@...n.tech>, Arthur Petukhovsky <arthur@...n.tech>,
 Stefan Radig <stefan@...n.tech>, Misha Sakhnov <misha@...n.tech>
Subject: Re: [PATCH v3 1/2] x86/mm: Handle alloc failure in phys_*_init()

On 6/11/25 15:16, Dave Hansen wrote:
> On 6/11/25 01:38, Em Sharnoff wrote:
>>> Could you please find a way to reduce the number of casts?
>> What do you think about changing the return for these functions to just 'int'
>> for errors?
> Fine with me. No reason to cram errno's into a physical address that's
> never used as a physical address.

Just realized paddr_last is actually used to set 'max_pfn_mapped'.
In init_memory_mapping():

> add_pfn_range_mapped(start >> PAGE_SHIFT, ret >> PAGE_SHIFT);

which in turn only uses it to update max_pfn_mapped:

> max_pfn_mapped = max(max_pfn_mapped, end_pfn)

This was added in cc6150321903 ("x86: account overlapped mappings in
max_pfn_mapped").

---

Some other options to reduce the number of casts:

1. Add helpers to do the '(void *)' casting for ERR_PTR, keeping everything
   else the same.
2. Change the phys_*_init() functions to return int, and directly update
   max_pfn_mapped from within them. They already call update_page_count(),
   maybe this is similar?
3. Change the phys_*_init() functions to return int, and calculate the
   expected paddr_last externally.

The third option I think is possible in theory, but probably too complicated
and fragile. (at a glance, there's complex emergent logic - but maybe someone
familiar with the code could make the case for something simple)

Thoughts?

Em