[PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled

[Peter Collingbourne]

On Thu, Sep 9, 2021 at 10:39 AM Catalin Marinas <catalin.marinas at arm.com> wrote:
>
> On Thu, Aug 26, 2021 at 08:07:48PM -0700, Peter Collingbourne wrote:
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index 923ee2ac85fd..e12ed892cde8 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -178,9 +178,9 @@ alternative_else_nop_endif
> >
> >       .macro mte_set_kernel_gcr, tmp, tmp2
> >  #ifdef CONFIG_KASAN_HW_TAGS
> > -alternative_if_not ARM64_MTE
> > +alternative_cb       kasan_hw_tags_enable
> >       b       1f
> > -alternative_else_nop_endif
> > +alternative_cb_end
> >       mov     \tmp, KERNEL_GCR_EL1
> >       msr_s   SYS_GCR_EL1, \tmp
> >  1:
> > @@ -188,10 +188,10 @@ alternative_else_nop_endif
> >       .endm
> >
> >       .macro mte_set_user_gcr, tsk, tmp, tmp2
> > -#ifdef CONFIG_ARM64_MTE
> > -alternative_if_not ARM64_MTE
> > +#ifdef CONFIG_KASAN_HW_TAGS
> > +alternative_cb       kasan_hw_tags_enable
> >       b       1f
> > -alternative_else_nop_endif
> > +alternative_cb_end
> >       ldr     \tmp, [\tsk, #THREAD_MTE_CTRL]
> >
> >       mte_set_gcr \tmp, \tmp2
> > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > index 9d314a3bad3b..0e8edb5d5861 100644
> > --- a/arch/arm64/kernel/mte.c
> > +++ b/arch/arm64/kernel/mte.c
> > @@ -162,7 +162,7 @@ void mte_check_tfsr_el1(void)
> >  }
> >  #endif
> >
> > -static void mte_update_sctlr_user(struct task_struct *task)
> > +static void mte_update_sctlr_user_and_gcr_excl(struct task_struct *task)
> >  {
> >       /*
> >        * This must be called with preemption disabled and can only be called
> > @@ -182,6 +182,24 @@ static void mte_update_sctlr_user(struct task_struct *task)
> >       else if (resolved_mte_tcf & MTE_CTRL_TCF_SYNC)
> >               sctlr |= SCTLR_EL1_TCF0_SYNC;
> >       task->thread.sctlr_user = sctlr;
> > +
> > +     /*
> > +      * SYS_GCR_EL1 will be set to current->thread.mte_ctrl value by
> > +      * mte_set_user_gcr() in kernel_exit, but only if KASAN is enabled.
> > +      */
> > +     if (!kasan_hw_tags_enabled())
> > +             write_sysreg_s(((mte_ctrl >> MTE_CTRL_GCR_USER_EXCL_SHIFT) &
> > +                             SYS_GCR_EL1_EXCL_MASK) | SYS_GCR_EL1_RRND,
> > +                            SYS_GCR_EL1);
> > +}
> > +
> > +void __init kasan_hw_tags_enable(struct alt_instr *alt, __le32 *origptr,
> > +                              __le32 *updptr, int nr_inst)
> > +{
> > +     BUG_ON(nr_inst != 1); /* Branch -> NOP */
> > +
> > +     if (kasan_hw_tags_enabled())
> > +             *updptr = cpu_to_le32(aarch64_insn_gen_nop());
> >  }
>
> A bit confusing: kasan_hw_tags_enabled() checks the kasan_flag_enabled
> static label. This is initialised via kasan_init_hw_tags() which is
> called from smp_prepare_boot_cpu() _after_ apply_boot_alternatives().
> Does this work when kasan is indeed enabled? Maybe I miss something.

Callback-based alternatives such as kasan_hw_tags_enable() are applied
via apply_alternatives_all(), which is called after the CPUs have been
brought up, because ARM64_CB_PATCH is not one of the boot
capabilities. Confirmed by setting breakpoints and observing the call
order:

(gdb) b kasan_hw_tags_enable
Breakpoint 1 at 0xffff80001173733c: file ../arch/arm64/kernel/mte.c, line 199.
(gdb) b kasan_init_hw_tags
Breakpoint 2 at 0xffff80001174c158: file ../mm/kasan/hw_tags.c, line 159.
(gdb) cont
Continuing.

Breakpoint 2, kasan_init_hw_tags () at ../mm/kasan/hw_tags.c:159
159 if (!system_supports_mte())
(gdb) bt
#0  kasan_init_hw_tags () at ../mm/kasan/hw_tags.c:159
#1  0xffff800011735804 in smp_prepare_boot_cpu () at
../arch/arm64/kernel/smp.c:466
#2  0xffff800011730844 in start_kernel () at ../init/main.c:937
#3  0xffff8000117303c8 in __primary_switched () at
../arch/arm64/kernel/head.S:467
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) cont
Continuing.

Breakpoint 1, kasan_hw_tags_enable (alt=<optimized out>,
origptr=<optimized out>, updptr=<optimized out>, nr_inst=<optimized
out>)
    at ../arch/arm64/kernel/mte.c:199
199 BUG_ON(nr_inst != 1); /* Branch -> NOP */
(gdb) bt
#0  kasan_hw_tags_enable (alt=<optimized out>, origptr=<optimized
out>, updptr=<optimized out>, nr_inst=<optimized out>)
    at ../arch/arm64/kernel/mte.c:199
#1  0xffff8000100232c4 in __apply_alternatives
(region=region at entry=0xffff80001211bd10, is_module=false,
    feature_mask=feature_mask at entry=0xffff80001211bd20) at
../arch/arm64/kernel/alternative.c:170
#2  0xffff800010023168 in __apply_alternatives_multi_stop
(unused=<optimized out>) at ../arch/arm64/kernel/alternative.c:218
#3  0xffff800010162e1c in multi_cpu_stop
(data=data at entry=0xffff80001000bd68 <_text+48488>) at
../kernel/stop_machine.c:240
#4  0xffff80001016351c in cpu_stopper_thread (cpu=<optimized out>) at
../kernel/stop_machine.c:511
#5  0xffff8000100ba294 in smpboot_thread_fn
(data=data at entry=0xffff000002494480) at ../kernel/smpboot.c:164
#6  0xffff8000100b36d4 in kthread (_create=0xffff000002494800) at
../kernel/kthread.c:319
#7  0xffff800010013eb4 in ret_from_fork () at ../arch/arm64/kernel/entry.S:798
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

Peter