linux-kernel - Re: [PATCH 2/4] x86/mm: harmonize return value of phys_pte

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] [day] [month] [year] [list]

Message-ID: <20251127143501.GAaShhlVTH5iQpCdPM@fat_crate.local>
Date: Thu, 27 Nov 2025 15:35:01 +0100
From: Borislav Petkov <bp@...en8.de>
To: Brendan Jackman <jackmanb@...gle.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] x86/mm: harmonize return value of phys_pte_init()

On Fri, Oct 03, 2025 at 04:56:42PM +0000, Brendan Jackman wrote:
> In the case that they encounter pre-existing mappings, all the other
> phys_*_init()s include those pre-mapped PFNs in the returned value.
> Excluding those PFNs only when they are mapped at 4K seems like an
> error. So make it consistent.
> 
> The other functions only include the existing mappings if the
> page_size_mask would have allowed creating those mappings.
> 4K pages can't be disabled by page_size_mask so that condition is not
> needed here; paddr_last can be assigned unconditionally before checking
> for existing mappings.
> 
> Signed-off-by: Brendan Jackman <jackmanb@...gle.com>
> ---
>  arch/x86/mm/init_64.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 9e45b371a6234b41bd7177b81b5d432341ae7214..968a5092dbd7ee3e7007fa0c769eff7d7ecb0ba3 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -492,6 +492,8 @@ phys_pte_init(pte_t *pte_page, unsigned long paddr, unsigned long paddr_end,
>  			continue;
>  		}
>  
> +		paddr_last = paddr_next;
> +
>  		/*
>  		 * We will re-use the existing mapping.
>  		 * Xen for example has some special requirements, like mapping

I don't understand: the other phys_*_init() things do:

		if (!XXX_none())

			...

			paddr_last = paddr_next;

while you've raised the assignment above that test.

Also "seems like an error" needs a lot more poking at because if it is an
error, then its incarnation must be really nasty and subtle or it is not, and
then we don't care. And it has been that way for a while now...

But maybe I'm not seeing it from the right angle...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette