linux-kernel - Re: [PATCH] arm64: mm: Move KPTI helpers to mmu.c

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: <bff6c831-68b0-4dbb-b54e-cbf349a942cb@arm.com>
Date: Wed, 10 Sep 2025 18:25:48 +0200
From: Kevin Brodsky <kevin.brodsky@....com>
To: Ryan Roberts <ryan.roberts@....com>, linux-arm-kernel@...ts.infradead.org
Cc: linux-kernel@...r.kernel.org,
 Anshuman Khandual <anshuman.khandual@....com>,
 Ard Biesheuvel <ardb@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
 Kees Cook <kees@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Suzuki K Poulose <suzuki.poulose@....com>, Will Deacon <will@...nel.org>,
 Yeoreum Yun <yeoreum.yun@....com>
Subject: Re: [PATCH] arm64: mm: Move KPTI helpers to mmu.c

On 10/09/2025 16:33, Ryan Roberts wrote:
> On 10/09/2025 11:44, Kevin Brodsky wrote:
>> create_kpti_ng_temp_pgd() is currently defined (as an alias) in
>> mmu.c without matching declaration in a header; instead cpufeature.c
>> makes its own declaration. This is clearly not pretty, and as commit
>> ceca927c86e6 ("arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc
>> function signature") showed, it also makes it very easy for the
>> prototypes to go out of sync.
>>
>> All this would be much simpler if kpti_install_ng_mappings() and
>> associated functions lived in mmu.c, where they logically belong.
>> This is what this patch does:
>> - Move kpti_install_ng_mappings() and associated functions from
>>   cpufeature.c to mmu.c, add a declaration to <asm/mmu.h>
>> - Make create_kpti_ng_temp_pgd() a static function that simply calls
>>   __create_pgd_mapping_locked() instead of aliasing it
>> - Mark all these functions __init
>> - Move __initdata after kpti_ng_temp_alloc (as suggested by
>>   checkpatch)
> This is a great clean up IMHO; that alias has caught me out a few times in the
> past when hacking in this area. And this code clearly belongs in mmu.c.

Good to hear!

>> Signed-off-by: Kevin Brodsky <kevin.brodsky@....com>
>> ---
>> Note: as things stand, create_kpti_ng_temp_pgd() could be removed,
>> but a separate patch [1] will make use of it to add an
>> assertion.
> I'd vote for removing it and just calling __create_pgd_mapping_locked() direct.
> The next version of the other patch can just rebase on top of yours and add the
> assert in __kpti_install_ng_mappings().

Oh yes good point, not sure why I didn't consider that! It clearly makes
more sense, I'll send a v2.

> Either way:
>
> Reviewed-by: Ryan Roberts <ryan.roberts@....com>
>
>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
>>
>> [...]
>>
>> +static pgprot_t __init kernel_exec_prot(void)
> nit: this change (to add __init) is unrelated; does it deserve it's own patch?

I thought I might as well sneak it in since it's still related to KPTI
and in the same block... But happy to split it off if that feels more
appropriate.

- Kevin