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: <CAMj1kXHDKGsaOn96zecba=-6Bib0SVYs=voMr5DLjWOn_qAqAQ@mail.gmail.com>
Date: Thu, 11 Sep 2025 08:24:44 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Anshuman Khandual <anshuman.khandual@....com>
Cc: Kevin Brodsky <kevin.brodsky@....com>, 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 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.

> Also
> is not bit cleaner as well ?

That is highly subjective, but personally, I think it only adds confusion.