[PATCH] powerpc64/kasan: Call kasan_early_init() after PACA initialised

[PATCH] powerpc64/kasan: Call kasan_early_init() after PACA initialised

[Benjamin Gray]

The KCOV handler __sanitizer_cov_trace_pc() uses the PACA, so initialise
the PACA first. This fixes a hang during boot when KASAN and KCOV are
both enabled, where the coverage tracer in kasan_early_init() tries to
access a field of the (currently null) PACA.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>

---

I tried annotating kasan_early_init() with 'notrace', but it still
seemed to hang. It would also be less robust, because kasan_early_init()
may in future call generic code that should keep coverage.
---
 arch/powerpc/kernel/head_64.S  | 3 ---
 arch/powerpc/kernel/setup_64.c | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index f132d8704263..21a78a849ca8 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -1004,9 +1004,6 @@ start_here_multiplatform:
 	 * and SLB setup before we turn on relocation.
 	 */
 
-#ifdef CONFIG_KASAN
-	bl	CFUNC(kasan_early_init)
-#endif
 	/* Restore parameters passed from prom_init/kexec */
 	mr	r3,r31
 	LOAD_REG_ADDR(r12, DOTSYM(early_setup))
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 246201d0d879..a3f5decbc041 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -369,6 +369,10 @@ void __init early_setup(unsigned long dt_ptr)
 
 	/* -------- printk is now safe to use ------- */
 
+#ifdef CONFIG_KASAN
+	kasan_early_init();
+#endif
+
 	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && (mfmsr() & MSR_HV))
 		enable_machine_check();
 
-- 
2.41.0

Re: [PATCH] powerpc64/kasan: Call kasan_early_init() after PACA initialised

[Benjamin Gray]

The inclusion of kasan_early_init() is conditional on CONFIG_KASAN, so

  if(IS_ENABLED(CONFIG_KASAN)) 
      kasan_early_init();

wouldn't work.

Re: [PATCH] powerpc64/kasan: Call kasan_early_init() after PACA initialised

[Christophe Leroy]

Le 07/07/2023 à 04:20, Benjamin Gray a écrit :
> The inclusion of kasan_early_init() is conditional on CONFIG_KASAN, so
> 
>    if(IS_ENABLED(CONFIG_KASAN))
>        kasan_early_init();
> 
> wouldn't work.

See my comment on the main patch, just do like kasan_init() and others 
in arch/powerpc/include/asm/kasan.h : Define a stub for when 
CONFIG_KASAN is not selected.

Re: [PATCH] powerpc64/kasan: Call kasan_early_init() after PACA initialised

[Christophe Leroy]

Le 07/07/2023 à 03:31, Benjamin Gray a écrit :
> The KCOV handler __sanitizer_cov_trace_pc() uses the PACA, so initialise
> the PACA first. This fixes a hang during boot when KASAN and KCOV are
> both enabled, where the coverage tracer in kasan_early_init() tries to
> access a field of the (currently null) PACA.

mm/kasan/Makefile has KCOV_INSTRUMENT := n
Should we add the same in arch/powerpc/mm/kasan/Makefile to be consistent ?

> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> 
> ---
> 
> I tried annotating kasan_early_init() with 'notrace', but it still
> seemed to hang. It would also be less robust, because kasan_early_init()
> may in future call generic code that should keep coverage.
> ---
>   arch/powerpc/kernel/head_64.S  | 3 ---
>   arch/powerpc/kernel/setup_64.c | 4 ++++
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index f132d8704263..21a78a849ca8 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -1004,9 +1004,6 @@ start_here_multiplatform:
>   	 * and SLB setup before we turn on relocation.
>   	 */
>   
> -#ifdef CONFIG_KASAN
> -	bl	CFUNC(kasan_early_init)
> -#endif
>   	/* Restore parameters passed from prom_init/kexec */
>   	mr	r3,r31
>   	LOAD_REG_ADDR(r12, DOTSYM(early_setup))
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 246201d0d879..a3f5decbc041 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -369,6 +369,10 @@ void __init early_setup(unsigned long dt_ptr)
>   
>   	/* -------- printk is now safe to use ------- */
>   
> +#ifdef CONFIG_KASAN
> +	kasan_early_init();
> +#endif

Don't #ifdef

In arch/powerpc/include/asm/kasan.h add a stub for kasan_early_init() 
just like already done for kasan_init(), kasan_mmu_init() and 
kasan_late_init().

> +
>   	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && (mfmsr() & MSR_HV))
>   		enable_machine_check();
>