linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
@ 2025-08-29 15:49 Kees Cook
  2025-08-29 15:59 ` Ard Biesheuvel
  2025-08-29 16:06 ` Mark Rutland
  0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2025-08-29 15:49 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Kees Cook, Catalin Marinas, Will Deacon, Oliver Upton,
	Anshuman Khandual, Yue Haibing, Mark Rutland, Marc Zyngier,
	Mark Brown, linux-arm-kernel, Ryan Roberts, Shameer Kolothum,
	Joey Gouly, Yeoreum Yun, James Morse, Hardevsinh Palaniya,
	Andrew Morton, Kevin Brodsky, David Hildenbrand, Zhenhua Huang,
	Lorenzo Stoakes, Dev Jain, Yicong Yang, linux-kernel,
	linux-hardening

Seen during KPTI initialization:

  CFI failure at create_kpti_ng_temp_pgd+0x124/0xce8 (target: kpti_ng_pgd_alloc+0x0/0x14; expected type: 0xd61b88b6)

The call site is alloc_init_pud() at arch/arm64/mm/mmu.c:

  pud_phys = pgtable_alloc(TABLE_PUD);

alloc_init_pud() has the prototype:

  static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
                             phys_addr_t phys, pgprot_t prot,
                             phys_addr_t (*pgtable_alloc)(enum pgtable_type),
                             int flags)

where the pgtable_alloc() prototype is declared.

The target (kpti_ng_pgd_alloc) is used in arch/arm64/kernel/cpufeature.c:

  create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), KPTI_NG_TEMP_VA,
                          PAGE_SIZE, PAGE_KERNEL, kpti_ng_pgd_alloc, 0);

which is an alias for __create_pgd_mapping_locked() with prototype:

  extern __alias(__create_pgd_mapping_locked)
  void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
                               unsigned long virt,
                               phys_addr_t size, pgprot_t prot,
                               phys_addr_t (*pgtable_alloc)(enum pgtable_type),
                               int flags);

__create_pgd_mapping_locked() passes the function pointer down:

  __create_pgd_mapping_locked() -> alloc_init_p4d() -> alloc_init_pud()

But the target function (kpti_ng_pgd_alloc) has the wrong signature:

  static phys_addr_t __init kpti_ng_pgd_alloc(int shift);

The "int" should be "enum pgtable_type".

To make "enum pgtable_type" available to cpufeature.c, move
enum pgtable_type definition from arch/arm64/mm/mmu.c to
arch/arm64/include/asm/mmu.h.

Adjust kpti_ng_pgd_alloc to use "enum pgtable_type" instead of "int".
The function behavior remains identical (parameter is unused).

Fixes: 47546a1912fc ("arm64: mm: install KPTI nG mappings with MMU enabled")
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Yue Haibing <yuehaibing@huawei.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: <linux-arm-kernel@lists.infradead.org>
---
 arch/arm64/include/asm/mmu.h   | 7 +++++++
 arch/arm64/kernel/cpufeature.c | 5 +++--
 arch/arm64/mm/mmu.c            | 7 -------
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 6e8aa8e72601..49f1a810df16 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -17,6 +17,13 @@
 #include <linux/refcount.h>
 #include <asm/cpufeature.h>
 
+enum pgtable_type {
+	TABLE_PTE,
+	TABLE_PMD,
+	TABLE_PUD,
+	TABLE_P4D,
+};
+
 typedef struct {
 	atomic64_t	id;
 #ifdef CONFIG_COMPAT
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9ad065f15f1d..e49d142a281f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -84,6 +84,7 @@
 #include <asm/hwcap.h>
 #include <asm/insn.h>
 #include <asm/kvm_host.h>
+#include <asm/mmu.h>
 #include <asm/mmu_context.h>
 #include <asm/mte.h>
 #include <asm/hypervisor.h>
@@ -1945,11 +1946,11 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
 extern
 void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
 			     phys_addr_t size, pgprot_t prot,
-			     phys_addr_t (*pgtable_alloc)(int), int flags);
+			     phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
 
 static phys_addr_t __initdata kpti_ng_temp_alloc;
 
-static phys_addr_t __init kpti_ng_pgd_alloc(int shift)
+static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
 {
 	kpti_ng_temp_alloc -= PAGE_SIZE;
 	return kpti_ng_temp_alloc;
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 34e5d78af076..183801520740 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -47,13 +47,6 @@
 #define NO_CONT_MAPPINGS	BIT(1)
 #define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
 
-enum pgtable_type {
-	TABLE_PTE,
-	TABLE_PMD,
-	TABLE_PUD,
-	TABLE_P4D,
-};
-
 u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
  2025-08-29 15:49 [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature Kees Cook
@ 2025-08-29 15:59 ` Ard Biesheuvel
  2025-08-29 16:06   ` Ryan Roberts
  2025-08-29 16:06 ` Mark Rutland
  1 sibling, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2025-08-29 15:59 UTC (permalink / raw)
  To: Kees Cook
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Anshuman Khandual,
	Yue Haibing, Mark Rutland, Marc Zyngier, Mark Brown,
	linux-arm-kernel, Ryan Roberts, Shameer Kolothum, Joey Gouly,
	Yeoreum Yun, James Morse, Hardevsinh Palaniya, Andrew Morton,
	Kevin Brodsky, David Hildenbrand, Zhenhua Huang, Lorenzo Stoakes,
	Dev Jain, Yicong Yang, linux-kernel, linux-hardening

On Fri, 29 Aug 2025 at 17:49, Kees Cook <kees@kernel.org> wrote:
>
> Seen during KPTI initialization:
>
>   CFI failure at create_kpti_ng_temp_pgd+0x124/0xce8 (target: kpti_ng_pgd_alloc+0x0/0x14; expected type: 0xd61b88b6)
>
> The call site is alloc_init_pud() at arch/arm64/mm/mmu.c:
>
>   pud_phys = pgtable_alloc(TABLE_PUD);
>
> alloc_init_pud() has the prototype:
>
>   static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
>                              phys_addr_t phys, pgprot_t prot,
>                              phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>                              int flags)
>
> where the pgtable_alloc() prototype is declared.
>
> The target (kpti_ng_pgd_alloc) is used in arch/arm64/kernel/cpufeature.c:
>
>   create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), KPTI_NG_TEMP_VA,
>                           PAGE_SIZE, PAGE_KERNEL, kpti_ng_pgd_alloc, 0);
>
> which is an alias for __create_pgd_mapping_locked() with prototype:
>
>   extern __alias(__create_pgd_mapping_locked)
>   void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>                                unsigned long virt,
>                                phys_addr_t size, pgprot_t prot,
>                                phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>                                int flags);
>
> __create_pgd_mapping_locked() passes the function pointer down:
>
>   __create_pgd_mapping_locked() -> alloc_init_p4d() -> alloc_init_pud()
>
> But the target function (kpti_ng_pgd_alloc) has the wrong signature:
>
>   static phys_addr_t __init kpti_ng_pgd_alloc(int shift);
>
> The "int" should be "enum pgtable_type".
>
> To make "enum pgtable_type" available to cpufeature.c, move
> enum pgtable_type definition from arch/arm64/mm/mmu.c to
> arch/arm64/include/asm/mmu.h.
>
> Adjust kpti_ng_pgd_alloc to use "enum pgtable_type" instead of "int".
> The function behavior remains identical (parameter is unused).
>
> Fixes: 47546a1912fc ("arm64: mm: install KPTI nG mappings with MMU enabled")

Shouldn't this be

Fixes: c64f46ee1377 ("arm64: mm: use enum to identify pgtable level
instead of *_SHIFT")


> Signed-off-by: Kees Cook <kees@kernel.org>

Acked-by: Ard Biesheuvel <ardb@kernel.org>


> ---
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Yue Haibing <yuehaibing@huawei.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: <linux-arm-kernel@lists.infradead.org>
> ---
>  arch/arm64/include/asm/mmu.h   | 7 +++++++
>  arch/arm64/kernel/cpufeature.c | 5 +++--
>  arch/arm64/mm/mmu.c            | 7 -------
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 6e8aa8e72601..49f1a810df16 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -17,6 +17,13 @@
>  #include <linux/refcount.h>
>  #include <asm/cpufeature.h>
>
> +enum pgtable_type {
> +       TABLE_PTE,
> +       TABLE_PMD,
> +       TABLE_PUD,
> +       TABLE_P4D,
> +};
> +
>  typedef struct {
>         atomic64_t      id;
>  #ifdef CONFIG_COMPAT
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9ad065f15f1d..e49d142a281f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -84,6 +84,7 @@
>  #include <asm/hwcap.h>
>  #include <asm/insn.h>
>  #include <asm/kvm_host.h>
> +#include <asm/mmu.h>
>  #include <asm/mmu_context.h>
>  #include <asm/mte.h>
>  #include <asm/hypervisor.h>
> @@ -1945,11 +1946,11 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
>  extern
>  void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
>                              phys_addr_t size, pgprot_t prot,
> -                            phys_addr_t (*pgtable_alloc)(int), int flags);
> +                            phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
>
>  static phys_addr_t __initdata kpti_ng_temp_alloc;
>
> -static phys_addr_t __init kpti_ng_pgd_alloc(int shift)
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
>  {
>         kpti_ng_temp_alloc -= PAGE_SIZE;
>         return kpti_ng_temp_alloc;
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 34e5d78af076..183801520740 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -47,13 +47,6 @@
>  #define NO_CONT_MAPPINGS       BIT(1)
>  #define NO_EXEC_MAPPINGS       BIT(2)  /* assumes FEAT_HPDS is not used */
>
> -enum pgtable_type {
> -       TABLE_PTE,
> -       TABLE_PMD,
> -       TABLE_PUD,
> -       TABLE_P4D,
> -};
> -
>  u64 kimage_voffset __ro_after_init;
>  EXPORT_SYMBOL(kimage_voffset);
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
  2025-08-29 15:59 ` Ard Biesheuvel
@ 2025-08-29 16:06   ` Ryan Roberts
  0 siblings, 0 replies; 6+ messages in thread
From: Ryan Roberts @ 2025-08-29 16:06 UTC (permalink / raw)
  To: Ard Biesheuvel, Kees Cook, Kevin Brodsky
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Anshuman Khandual,
	Yue Haibing, Mark Rutland, Marc Zyngier, Mark Brown,
	linux-arm-kernel, Shameer Kolothum, Joey Gouly, Yeoreum Yun,
	James Morse, Hardevsinh Palaniya, Andrew Morton, Kevin Brodsky,
	David Hildenbrand, Zhenhua Huang, Lorenzo Stoakes, Dev Jain,
	Yicong Yang, linux-kernel, linux-hardening

+ Kevin Broadsky.

On 29/08/2025 16:59, Ard Biesheuvel wrote:
> On Fri, 29 Aug 2025 at 17:49, Kees Cook <kees@kernel.org> wrote:
>>
>> Seen during KPTI initialization:
>>
>>   CFI failure at create_kpti_ng_temp_pgd+0x124/0xce8 (target: kpti_ng_pgd_alloc+0x0/0x14; expected type: 0xd61b88b6)
>>
>> The call site is alloc_init_pud() at arch/arm64/mm/mmu.c:
>>
>>   pud_phys = pgtable_alloc(TABLE_PUD);
>>
>> alloc_init_pud() has the prototype:
>>
>>   static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
>>                              phys_addr_t phys, pgprot_t prot,
>>                              phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>>                              int flags)
>>
>> where the pgtable_alloc() prototype is declared.
>>
>> The target (kpti_ng_pgd_alloc) is used in arch/arm64/kernel/cpufeature.c:
>>
>>   create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), KPTI_NG_TEMP_VA,
>>                           PAGE_SIZE, PAGE_KERNEL, kpti_ng_pgd_alloc, 0);
>>
>> which is an alias for __create_pgd_mapping_locked() with prototype:
>>
>>   extern __alias(__create_pgd_mapping_locked)
>>   void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>>                                unsigned long virt,
>>                                phys_addr_t size, pgprot_t prot,
>>                                phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>>                                int flags);
>>
>> __create_pgd_mapping_locked() passes the function pointer down:
>>
>>   __create_pgd_mapping_locked() -> alloc_init_p4d() -> alloc_init_pud()
>>
>> But the target function (kpti_ng_pgd_alloc) has the wrong signature:
>>
>>   static phys_addr_t __init kpti_ng_pgd_alloc(int shift);
>>
>> The "int" should be "enum pgtable_type".
>>
>> To make "enum pgtable_type" available to cpufeature.c, move
>> enum pgtable_type definition from arch/arm64/mm/mmu.c to
>> arch/arm64/include/asm/mmu.h.
>>
>> Adjust kpti_ng_pgd_alloc to use "enum pgtable_type" instead of "int".
>> The function behavior remains identical (parameter is unused).
>>
>> Fixes: 47546a1912fc ("arm64: mm: install KPTI nG mappings with MMU enabled")
> 
> Shouldn't this be
> 
> Fixes: c64f46ee1377 ("arm64: mm: use enum to identify pgtable level
> instead of *_SHIFT")

You beat me to it; agreed. I've added Kevin, who made that change incase there
are any other subtlety.

> 
> 
>> Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

> 
> 
>> ---
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Oliver Upton <oliver.upton@linux.dev>
>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Yue Haibing <yuehaibing@huawei.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: <linux-arm-kernel@lists.infradead.org>
>> ---
>>  arch/arm64/include/asm/mmu.h   | 7 +++++++
>>  arch/arm64/kernel/cpufeature.c | 5 +++--
>>  arch/arm64/mm/mmu.c            | 7 -------
>>  3 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>> index 6e8aa8e72601..49f1a810df16 100644
>> --- a/arch/arm64/include/asm/mmu.h
>> +++ b/arch/arm64/include/asm/mmu.h
>> @@ -17,6 +17,13 @@
>>  #include <linux/refcount.h>
>>  #include <asm/cpufeature.h>
>>
>> +enum pgtable_type {
>> +       TABLE_PTE,
>> +       TABLE_PMD,
>> +       TABLE_PUD,
>> +       TABLE_P4D,
>> +};
>> +
>>  typedef struct {
>>         atomic64_t      id;
>>  #ifdef CONFIG_COMPAT
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 9ad065f15f1d..e49d142a281f 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -84,6 +84,7 @@
>>  #include <asm/hwcap.h>
>>  #include <asm/insn.h>
>>  #include <asm/kvm_host.h>
>> +#include <asm/mmu.h>
>>  #include <asm/mmu_context.h>
>>  #include <asm/mte.h>
>>  #include <asm/hypervisor.h>
>> @@ -1945,11 +1946,11 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
>>  extern
>>  void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
>>                              phys_addr_t size, pgprot_t prot,
>> -                            phys_addr_t (*pgtable_alloc)(int), int flags);
>> +                            phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
>>
>>  static phys_addr_t __initdata kpti_ng_temp_alloc;
>>
>> -static phys_addr_t __init kpti_ng_pgd_alloc(int shift)
>> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
>>  {
>>         kpti_ng_temp_alloc -= PAGE_SIZE;
>>         return kpti_ng_temp_alloc;
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 34e5d78af076..183801520740 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -47,13 +47,6 @@
>>  #define NO_CONT_MAPPINGS       BIT(1)
>>  #define NO_EXEC_MAPPINGS       BIT(2)  /* assumes FEAT_HPDS is not used */
>>
>> -enum pgtable_type {
>> -       TABLE_PTE,
>> -       TABLE_PMD,
>> -       TABLE_PUD,
>> -       TABLE_P4D,
>> -};
>> -
>>  u64 kimage_voffset __ro_after_init;
>>  EXPORT_SYMBOL(kimage_voffset);
>>
>> --
>> 2.34.1
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
  2025-08-29 15:49 [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature Kees Cook
  2025-08-29 15:59 ` Ard Biesheuvel
@ 2025-08-29 16:06 ` Mark Rutland
  2025-08-29 16:41   ` Kees Cook
  2025-09-01  7:33   ` Kevin Brodsky
  1 sibling, 2 replies; 6+ messages in thread
From: Mark Rutland @ 2025-08-29 16:06 UTC (permalink / raw)
  To: Kees Cook, Kevin Brodsky
  Cc: Ard Biesheuvel, Catalin Marinas, Will Deacon, Oliver Upton,
	Anshuman Khandual, Yue Haibing, Marc Zyngier, Mark Brown,
	linux-arm-kernel, Ryan Roberts, Shameer Kolothum, Joey Gouly,
	Yeoreum Yun, James Morse, Hardevsinh Palaniya, Andrew Morton,
	David Hildenbrand, Zhenhua Huang, Lorenzo Stoakes, Dev Jain,
	Yicong Yang, linux-kernel, linux-hardening

On Fri, Aug 29, 2025 at 08:49:21AM -0700, Kees Cook wrote:
> Seen during KPTI initialization:
> 
>   CFI failure at create_kpti_ng_temp_pgd+0x124/0xce8 (target: kpti_ng_pgd_alloc+0x0/0x14; expected type: 0xd61b88b6)
> 
> The call site is alloc_init_pud() at arch/arm64/mm/mmu.c:
> 
>   pud_phys = pgtable_alloc(TABLE_PUD);
> 
> alloc_init_pud() has the prototype:
> 
>   static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
>                              phys_addr_t phys, pgprot_t prot,
>                              phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>                              int flags)
> 
> where the pgtable_alloc() prototype is declared.
> 
> The target (kpti_ng_pgd_alloc) is used in arch/arm64/kernel/cpufeature.c:
> 
>   create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc), KPTI_NG_TEMP_VA,
>                           PAGE_SIZE, PAGE_KERNEL, kpti_ng_pgd_alloc, 0);
> 
> which is an alias for __create_pgd_mapping_locked() with prototype:
> 
>   extern __alias(__create_pgd_mapping_locked)
>   void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>                                unsigned long virt,
>                                phys_addr_t size, pgprot_t prot,
>                                phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>                                int flags);
> 
> __create_pgd_mapping_locked() passes the function pointer down:
> 
>   __create_pgd_mapping_locked() -> alloc_init_p4d() -> alloc_init_pud()
> 
> But the target function (kpti_ng_pgd_alloc) has the wrong signature:
> 
>   static phys_addr_t __init kpti_ng_pgd_alloc(int shift);
> 
> The "int" should be "enum pgtable_type".
> 
> To make "enum pgtable_type" available to cpufeature.c, move
> enum pgtable_type definition from arch/arm64/mm/mmu.c to
> arch/arm64/include/asm/mmu.h.
> 
> Adjust kpti_ng_pgd_alloc to use "enum pgtable_type" instead of "int".
> The function behavior remains identical (parameter is unused).
> 
> Fixes: 47546a1912fc ("arm64: mm: install KPTI nG mappings with MMU enabled")

That doesn't look right; that commit is from June 2022, and we only
introduced enum pgtable_type in May 2025 in commit:

  c64f46ee13779616 ("arm64: mm: use enum to identify pgtable level instead of *_SHIFT")

... which landed in v6.16.

AFAICT, that's the commit which broke things.

The actual fix looks fine, though I suspect we should move more of this
logic into mmu.c.

Mark.

> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Yue Haibing <yuehaibing@huawei.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: <linux-arm-kernel@lists.infradead.org>
> ---
>  arch/arm64/include/asm/mmu.h   | 7 +++++++
>  arch/arm64/kernel/cpufeature.c | 5 +++--
>  arch/arm64/mm/mmu.c            | 7 -------
>  3 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 6e8aa8e72601..49f1a810df16 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -17,6 +17,13 @@
>  #include <linux/refcount.h>
>  #include <asm/cpufeature.h>
>  
> +enum pgtable_type {
> +	TABLE_PTE,
> +	TABLE_PMD,
> +	TABLE_PUD,
> +	TABLE_P4D,
> +};
> +
>  typedef struct {
>  	atomic64_t	id;
>  #ifdef CONFIG_COMPAT
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9ad065f15f1d..e49d142a281f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -84,6 +84,7 @@
>  #include <asm/hwcap.h>
>  #include <asm/insn.h>
>  #include <asm/kvm_host.h>
> +#include <asm/mmu.h>
>  #include <asm/mmu_context.h>
>  #include <asm/mte.h>
>  #include <asm/hypervisor.h>
> @@ -1945,11 +1946,11 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
>  extern
>  void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
>  			     phys_addr_t size, pgprot_t prot,
> -			     phys_addr_t (*pgtable_alloc)(int), int flags);
> +			     phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
>  
>  static phys_addr_t __initdata kpti_ng_temp_alloc;
>  
> -static phys_addr_t __init kpti_ng_pgd_alloc(int shift)
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
>  {
>  	kpti_ng_temp_alloc -= PAGE_SIZE;
>  	return kpti_ng_temp_alloc;
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 34e5d78af076..183801520740 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -47,13 +47,6 @@
>  #define NO_CONT_MAPPINGS	BIT(1)
>  #define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
>  
> -enum pgtable_type {
> -	TABLE_PTE,
> -	TABLE_PMD,
> -	TABLE_PUD,
> -	TABLE_P4D,
> -};
> -
>  u64 kimage_voffset __ro_after_init;
>  EXPORT_SYMBOL(kimage_voffset);
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
  2025-08-29 16:06 ` Mark Rutland
@ 2025-08-29 16:41   ` Kees Cook
  2025-09-01  7:33   ` Kevin Brodsky
  1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2025-08-29 16:41 UTC (permalink / raw)
  To: Mark Rutland, Kevin Brodsky
  Cc: Ard Biesheuvel, Catalin Marinas, Will Deacon, Oliver Upton,
	Anshuman Khandual, Yue Haibing, Marc Zyngier, Mark Brown,
	linux-arm-kernel, Ryan Roberts, Shameer Kolothum, Joey Gouly,
	Yeoreum Yun, James Morse, Hardevsinh Palaniya, Andrew Morton,
	David Hildenbrand, Zhenhua Huang, Lorenzo Stoakes, Dev Jain,
	Yicong Yang, linux-kernel, linux-hardening



On August 29, 2025 12:06:17 PM EDT, Mark Rutland <mark.rutland@arm.com> wrote:
>On Fri, Aug 29, 2025 at 08:49:21AM -0700, Kees Cook wrote:
>> Fixes: 47546a1912fc ("arm64: mm: install KPTI nG mappings with MMU enabled")
>
>That doesn't look right; that commit is from June 2022, and we only
>introduced enum pgtable_type in May 2025 in commit:
>
>  c64f46ee13779616 ("arm64: mm: use enum to identify pgtable level instead of *_SHIFT")
>
>... which landed in v6.16.
>
>AFAICT, that's the commit which broke things.

Oops, yes, I didn't look close enough. Want a v2 with that fixed up?


-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature
  2025-08-29 16:06 ` Mark Rutland
  2025-08-29 16:41   ` Kees Cook
@ 2025-09-01  7:33   ` Kevin Brodsky
  1 sibling, 0 replies; 6+ messages in thread
From: Kevin Brodsky @ 2025-09-01  7:33 UTC (permalink / raw)
  To: Mark Rutland, Kees Cook
  Cc: Ard Biesheuvel, Catalin Marinas, Will Deacon, Oliver Upton,
	Anshuman Khandual, Yue Haibing, Marc Zyngier, Mark Brown,
	linux-arm-kernel, Ryan Roberts, Shameer Kolothum, Joey Gouly,
	Yeoreum Yun, James Morse, Hardevsinh Palaniya, Andrew Morton,
	David Hildenbrand, Zhenhua Huang, Lorenzo Stoakes, Dev Jain,
	Yicong Yang, linux-kernel, linux-hardening

On 29/08/2025 18:06, Mark Rutland wrote:
> The actual fix looks fine, though I suspect we should move more of this
> logic into mmu.c.

Agreed, I'll look into that. Having two separate declarations in
cpufeature.c and mmu.c is ugly and leads to the kind of mistake that I made.

The fix looks perfectly fine FWIW - not adding my Reviewed-by as I saw
Catalin took v2 already :)

- Kevin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-09-01  7:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 15:49 [PATCH] arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc function signature Kees Cook
2025-08-29 15:59 ` Ard Biesheuvel
2025-08-29 16:06   ` Ryan Roberts
2025-08-29 16:06 ` Mark Rutland
2025-08-29 16:41   ` Kees Cook
2025-09-01  7:33   ` Kevin Brodsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).