[PATCH] arm64: fix KASAN_INLINE

[PATCH] arm64: fix KASAN_INLINE

[Mark Rutland]

Since commit:

  a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")

Kernels built with KASAN_INLINE=y die early in boot before producing any
console output. This is because the accesses made to the FDT (e.g. in
generic string processing functions) are instrumented with KASAN, and
with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus
shadow VA, resulting in a data abort.

This patch fixes this by reverting commits:

  7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers")
  bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map")

... and using the TTBR1 fixmap mapping of the FDT.

Note that due to a later commit:

  b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1")

... which altered the prototype of init_feature_override() (and
invocation from head.S), commit bd0c3fa21878b6d0 does not revert
cleanly, and I've fixed that up manually.

Fixes: a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/setup.h     |  3 +++
 arch/arm64/kernel/head.S           |  5 +++--
 arch/arm64/kernel/idreg-override.c | 17 +++++++++++------
 arch/arm64/kernel/setup.c          | 15 +++++++++++++++
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
index 5f147a418281..6437df661700 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -5,6 +5,9 @@
 
 #include <uapi/asm/setup.h>
 
+void *get_early_fdt_ptr(void);
+void early_fdt_map(u64 dt_phys);
+
 /*
  * These two variables are used in the head.S file.
  */
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 29d641290293..cefe6a73ee54 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -456,8 +456,9 @@ SYM_FUNC_START_LOCAL(__primary_switched)
 #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
 	bl	kasan_early_init
 #endif
-	mov	x0, x22				// pass FDT address in x0
-	mov	x1, x20				// pass the full boot status
+	mov	x0, x21				// pass FDT address in x0
+	bl	early_fdt_map			// Try mapping the FDT early
+	mov	x0, x20				// pass the full boot status
 	bl	init_feature_override		// Parse cpu feature overrides
 	mov	x0, x20
 	bl	finalise_el2			// Prefer VHE if possible
diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
index 42883657f711..1b0542c69738 100644
--- a/arch/arm64/kernel/idreg-override.c
+++ b/arch/arm64/kernel/idreg-override.c
@@ -262,11 +262,16 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases)
 	} while (1);
 }
 
-static __init const u8 *get_bootargs_cmdline(const void *fdt)
+static __init const u8 *get_bootargs_cmdline(void)
 {
 	const u8 *prop;
+	void *fdt;
 	int node;
 
+	fdt = get_early_fdt_ptr();
+	if (!fdt)
+		return NULL;
+
 	node = fdt_path_offset(fdt, "/chosen");
 	if (node < 0)
 		return NULL;
@@ -278,9 +283,9 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt)
 	return strlen(prop) ? prop : NULL;
 }
 
-static __init void parse_cmdline(const void *fdt)
+static __init void parse_cmdline(void)
 {
-	const u8 *prop = get_bootargs_cmdline(fdt);
+	const u8 *prop = get_bootargs_cmdline();
 
 	if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !prop)
 		__parse_cmdline(CONFIG_CMDLINE, true);
@@ -290,9 +295,9 @@ static __init void parse_cmdline(const void *fdt)
 }
 
 /* Keep checkers quiet */
-void init_feature_override(const void *fdt, u64 boot_status);
+void init_feature_override(u64 boot_status);
 
-asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status)
+asmlinkage void __init init_feature_override(u64 boot_status)
 {
 	int i;
 
@@ -305,7 +310,7 @@ asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status)
 
 	__boot_status = boot_status;
 
-	parse_cmdline(fdt);
+	parse_cmdline();
 
 	for (i = 0; i < ARRAY_SIZE(regs); i++) {
 		if (regs[i]->override)
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index d0e6c7a291da..fea3223704b6 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -163,6 +163,21 @@ static void __init smp_build_mpidr_hash(void)
 		pr_warn("Large number of MPIDR hash buckets detected\n");
 }
 
+static void *early_fdt_ptr __initdata;
+
+void __init *get_early_fdt_ptr(void)
+{
+	return early_fdt_ptr;
+}
+
+asmlinkage void __init early_fdt_map(u64 dt_phys)
+{
+	int fdt_size;
+
+	early_fixmap_init();
+	early_fdt_ptr = fixmap_remap_fdt(dt_phys, &fdt_size, PAGE_KERNEL);
+}
+
 static void __init setup_machine_fdt(phys_addr_t dt_phys)
 {
 	int size;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: fix KASAN_INLINE

[Catalin Marinas]

On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote:
> Since commit:
> 
>   a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> 
> Kernels built with KASAN_INLINE=y die early in boot before producing any
> console output. This is because the accesses made to the FDT (e.g. in
> generic string processing functions) are instrumented with KASAN, and
> with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus
> shadow VA, resulting in a data abort.
> 
> This patch fixes this by reverting commits:
> 
>   7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers")
>   bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map")
> 
> ... and using the TTBR1 fixmap mapping of the FDT.
> 
> Note that due to a later commit:
> 
>   b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1")
> 
> ... which altered the prototype of init_feature_override() (and
> invocation from head.S), commit bd0c3fa21878b6d0 does not revert
> cleanly, and I've fixed that up manually.
> 
> Fixes: a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>

I'll leave this to Will to pick since the fixed commit is only in -next.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: fix KASAN_INLINE

[Will Deacon]

On Wed, 13 Jul 2022 15:09:49 +0100, Mark Rutland wrote:
> Since commit:
> 
>   a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> 
> Kernels built with KASAN_INLINE=y die early in boot before producing any
> console output. This is because the accesses made to the FDT (e.g. in
> generic string processing functions) are instrumented with KASAN, and
> with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus
> shadow VA, resulting in a data abort.
> 
> [...]

Applied to arm64 (for-next/boot), thanks!

[1/1] arm64: fix KASAN_INLINE
      https://git.kernel.org/arm64/c/c2cbc16df707

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: fix KASAN_INLINE

[Mark Rutland]

On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote:
> Since commit:
> 
>   a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> 
> Kernels built with KASAN_INLINE=y die early in boot before producing any
> console output. This is because the accesses made to the FDT (e.g. in
> generic string processing functions) are instrumented with KASAN, and
> with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus
> shadow VA, resulting in a data abort.
> 
> This patch fixes this by reverting commits:
> 
>   7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers")
>   bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map")
> 
> ... and using the TTBR1 fixmap mapping of the FDT.
> 
> Note that due to a later commit:
> 
>   b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1")
> 
> ... which altered the prototype of init_feature_override() (and
> invocation from head.S), commit bd0c3fa21878b6d0 does not revert
> cleanly, and I've fixed that up manually.
> 

Whoops; this was meant to have:

  Signed-off-by: Mark Rutland <mark.rutland@arm.com>

... but I somehow messed that up.

Will, are you happy to fold that in?

Mark.

> Fixes: a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/setup.h     |  3 +++
>  arch/arm64/kernel/head.S           |  5 +++--
>  arch/arm64/kernel/idreg-override.c | 17 +++++++++++------
>  arch/arm64/kernel/setup.c          | 15 +++++++++++++++
>  4 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> index 5f147a418281..6437df661700 100644
> --- a/arch/arm64/include/asm/setup.h
> +++ b/arch/arm64/include/asm/setup.h
> @@ -5,6 +5,9 @@
>  
>  #include <uapi/asm/setup.h>
>  
> +void *get_early_fdt_ptr(void);
> +void early_fdt_map(u64 dt_phys);
> +
>  /*
>   * These two variables are used in the head.S file.
>   */
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 29d641290293..cefe6a73ee54 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -456,8 +456,9 @@ SYM_FUNC_START_LOCAL(__primary_switched)
>  #if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
>  	bl	kasan_early_init
>  #endif
> -	mov	x0, x22				// pass FDT address in x0
> -	mov	x1, x20				// pass the full boot status
> +	mov	x0, x21				// pass FDT address in x0
> +	bl	early_fdt_map			// Try mapping the FDT early
> +	mov	x0, x20				// pass the full boot status
>  	bl	init_feature_override		// Parse cpu feature overrides
>  	mov	x0, x20
>  	bl	finalise_el2			// Prefer VHE if possible
> diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
> index 42883657f711..1b0542c69738 100644
> --- a/arch/arm64/kernel/idreg-override.c
> +++ b/arch/arm64/kernel/idreg-override.c
> @@ -262,11 +262,16 @@ static __init void __parse_cmdline(const char *cmdline, bool parse_aliases)
>  	} while (1);
>  }
>  
> -static __init const u8 *get_bootargs_cmdline(const void *fdt)
> +static __init const u8 *get_bootargs_cmdline(void)
>  {
>  	const u8 *prop;
> +	void *fdt;
>  	int node;
>  
> +	fdt = get_early_fdt_ptr();
> +	if (!fdt)
> +		return NULL;
> +
>  	node = fdt_path_offset(fdt, "/chosen");
>  	if (node < 0)
>  		return NULL;
> @@ -278,9 +283,9 @@ static __init const u8 *get_bootargs_cmdline(const void *fdt)
>  	return strlen(prop) ? prop : NULL;
>  }
>  
> -static __init void parse_cmdline(const void *fdt)
> +static __init void parse_cmdline(void)
>  {
> -	const u8 *prop = get_bootargs_cmdline(fdt);
> +	const u8 *prop = get_bootargs_cmdline();
>  
>  	if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !prop)
>  		__parse_cmdline(CONFIG_CMDLINE, true);
> @@ -290,9 +295,9 @@ static __init void parse_cmdline(const void *fdt)
>  }
>  
>  /* Keep checkers quiet */
> -void init_feature_override(const void *fdt, u64 boot_status);
> +void init_feature_override(u64 boot_status);
>  
> -asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status)
> +asmlinkage void __init init_feature_override(u64 boot_status)
>  {
>  	int i;
>  
> @@ -305,7 +310,7 @@ asmlinkage void __init init_feature_override(const void *fdt, u64 boot_status)
>  
>  	__boot_status = boot_status;
>  
> -	parse_cmdline(fdt);
> +	parse_cmdline();
>  
>  	for (i = 0; i < ARRAY_SIZE(regs); i++) {
>  		if (regs[i]->override)
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index d0e6c7a291da..fea3223704b6 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -163,6 +163,21 @@ static void __init smp_build_mpidr_hash(void)
>  		pr_warn("Large number of MPIDR hash buckets detected\n");
>  }
>  
> +static void *early_fdt_ptr __initdata;
> +
> +void __init *get_early_fdt_ptr(void)
> +{
> +	return early_fdt_ptr;
> +}
> +
> +asmlinkage void __init early_fdt_map(u64 dt_phys)
> +{
> +	int fdt_size;
> +
> +	early_fixmap_init();
> +	early_fdt_ptr = fixmap_remap_fdt(dt_phys, &fdt_size, PAGE_KERNEL);
> +}
> +
>  static void __init setup_machine_fdt(phys_addr_t dt_phys)
>  {
>  	int size;
> -- 
> 2.30.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: fix KASAN_INLINE

[Will Deacon]

On Wed, Jul 20, 2022 at 03:53:41PM +0100, Mark Rutland wrote:
> On Wed, Jul 13, 2022 at 03:09:49PM +0100, Mark Rutland wrote:
> > Since commit:
> > 
> >   a004393f45d9a55e ("arm64: idreg-override: use early FDT mapping in ID map")
> > 
> > Kernels built with KASAN_INLINE=y die early in boot before producing any
> > console output. This is because the accesses made to the FDT (e.g. in
> > generic string processing functions) are instrumented with KASAN, and
> > with KASAN_INLINE=y any access to an address in TTBR0 results in a bogus
> > shadow VA, resulting in a data abort.
> > 
> > This patch fixes this by reverting commits:
> > 
> >   7559d9f97581654f ("arm64: setup: drop early FDT pointer helpers")
> >   bd0c3fa21878b6d0 ("arm64: idreg-override: use early FDT mapping in ID map")
> > 
> > ... and using the TTBR1 fixmap mapping of the FDT.
> > 
> > Note that due to a later commit:
> > 
> >   b65e411d6cc2f12a ("arm64: Save state of HCR_EL2.E2H before switch to EL1")
> > 
> > ... which altered the prototype of init_feature_override() (and
> > invocation from head.S), commit bd0c3fa21878b6d0 does not revert
> > cleanly, and I've fixed that up manually.
> > 
> 
> Whoops; this was meant to have:
> 
>   Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> 
> ... but I somehow messed that up.
> 
> Will, are you happy to fold that in?

Thanks, yes, I'll add this now.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel