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

Message-ID: <eb8bd2da-48be-4c4e-bf52-48ea1718e02b@arm.com>
Date: Thu, 11 Sep 2025 11:10:09 +0200
From: Kevin Brodsky <kevin.brodsky@....com>
To: Ard Biesheuvel <ardb@...nel.org>,
 Anshuman Khandual <anshuman.khandual@....com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Catalin Marinas <catalin.marinas@....com>, Kees Cook <kees@...nel.org>,
 Mark Rutland <mark.rutland@....com>, Ryan Roberts <ryan.roberts@....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 11/09/2025 08:24, Ard Biesheuvel wrote:
> On Thu, 11 Sept 2025 at 08:18, Anshuman Khandual
> <anshuman.khandual@....com> wrote:
>>
>>
>> On 11/09/25 11:38 AM, Ard Biesheuvel wrote:
>>> On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual
>>> <anshuman.khandual@....com> wrote:
>>>>
>>>>
>>>> On 10/09/25 4:14 PM, 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)
>>>>>
>>>>> 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.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
>>>>> ---
>>>>> Cc: Anshuman Khandual <anshuman.khandual@....com>
>>>>> Cc: Ard Biesheuvel <ardb@...nel.org>
>>>>> Cc: Catalin Marinas <catalin.marinas@....com>
>>>>> Cc: Kees Cook <kees@...nel.org>,
>>>>> Cc: Mark Rutland <mark.rutland@....com>
>>>>> Cc: Ryan Roberts <ryan.roberts@....com>
>>>>> Cc: Suzuki K Poulose <suzuki.poulose@....com>
>>>>> Cc: Will Deacon <will@...nel.org>
>>>>> Cc: Yeoreum Yun <yeoreum.yun@....com>
>>>>> ---
>>>>>  arch/arm64/include/asm/mmu.h   |   6 ++
>>>>>  arch/arm64/kernel/cpufeature.c |  97 ------------------------------
>>>>>  arch/arm64/mm/mmu.c            | 106 ++++++++++++++++++++++++++++++---
>>>>>  3 files changed, 103 insertions(+), 106 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>>>>> index 49f1a810df16..624edd6c4964 100644
>>>>> --- a/arch/arm64/include/asm/mmu.h
>>>>> +++ b/arch/arm64/include/asm/mmu.h
>>>>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
>>>>>       return true;
>>>>>  }
>>>>>
>>>>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>>>> +void kpti_install_ng_mappings(void);
>>>> Could the declarations be moved here instead ?
>>> Why?
>> To avoid both typedef and external instance declaration in the C
>> code even though there is just a single call site in there.
> But why would we want to avoid those in the first place?
>
> Moving these into mmu.h pollutes the global namespace with
> declarations that must never be used outside of
> __kpti_install_ng_mappings() to begin with.

That makes sense to me - no need for these declarations to be in a
header as they're implementation details of
__kpti_install_ng_mappings(). What is more common is to have them at the
top-level in the .c file instead of inside a function, but I think
either way is fine.

- Kevin