Message ID | 20230707013110.215693-1-bgray@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | powerpc64/kasan: Call kasan_early_init() after PACA initialised | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
The inclusion of kasan_early_init() is conditional on CONFIG_KASAN, so if(IS_ENABLED(CONFIG_KASAN)) kasan_early_init(); wouldn't work.
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(); >
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.
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();
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(-)