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

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

[Peter Collingbourne]

It is not necessary to write to GCR_EL1 on every kernel entry and
exit when HW tag-based KASAN is disabled because the kernel will not
execute any IRG instructions in that mode. Since accessing GCR_EL1
can be expensive on some microarchitectures, avoid doing so by moving
the access to task switch when HW tag-based KASAN is disabled.

Signed-off-by: Peter Collingbourne <pcc@google.com>
---
 arch/arm64/kernel/entry.S | 10 +++++-----
 arch/arm64/kernel/mte.c   | 24 +++++++++++++++++++++---
 include/linux/kasan.h     |  9 +++++++--
 3 files changed, 33 insertions(+), 10 deletions(-)

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());
 }
 
 void mte_thread_init_user(void)
@@ -199,7 +217,7 @@ void mte_thread_init_user(void)
 
 void mte_thread_switch(struct task_struct *next)
 {
-	mte_update_sctlr_user(next);
+	mte_update_sctlr_user_and_gcr_excl(next);
 
 	/*
 	 * Check if an async tag exception occurred at EL1.
@@ -244,7 +262,7 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
 	task->thread.mte_ctrl = mte_ctrl;
 	if (task == current) {
 		preempt_disable();
-		mte_update_sctlr_user(task);
+		mte_update_sctlr_user_and_gcr_excl(task);
 		update_sctlr_el1(task->thread.sctlr_user);
 		preempt_enable();
 	}
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index dd874a1ee862..de5f5913374d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -89,7 +89,7 @@ static __always_inline bool kasan_enabled(void)
 	return static_branch_likely(&kasan_flag_enabled);
 }
 
-static inline bool kasan_has_integrated_init(void)
+static inline bool kasan_hw_tags_enabled(void)
 {
 	return kasan_enabled();
 }
@@ -104,7 +104,7 @@ static inline bool kasan_enabled(void)
 	return IS_ENABLED(CONFIG_KASAN);
 }
 
-static inline bool kasan_has_integrated_init(void)
+static inline bool kasan_hw_tags_enabled(void)
 {
 	return false;
 }
@@ -125,6 +125,11 @@ static __always_inline void kasan_free_pages(struct page *page,
 
 #endif /* CONFIG_KASAN_HW_TAGS */
 
+static inline bool kasan_has_integrated_init(void)
+{
+	return kasan_hw_tags_enabled();
+}
+
 #ifdef CONFIG_KASAN
 
 struct kasan_cache {
-- 
2.33.0.259.gc128427fd7-goog


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

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

[Andrey Konovalov]

On Fri, Aug 27, 2021 at 5:07 AM Peter Collingbourne <pcc@google.com> wrote:
>
> It is not necessary to write to GCR_EL1 on every kernel entry and
> exit when HW tag-based KASAN is disabled because the kernel will not
> execute any IRG instructions in that mode. Since accessing GCR_EL1
> can be expensive on some microarchitectures, avoid doing so by moving
> the access to task switch when HW tag-based KASAN is disabled.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> ---
>  arch/arm64/kernel/entry.S | 10 +++++-----
>  arch/arm64/kernel/mte.c   | 24 +++++++++++++++++++++---
>  include/linux/kasan.h     |  9 +++++++--
>  3 files changed, 33 insertions(+), 10 deletions(-)
>
> 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());
>  }
>
>  void mte_thread_init_user(void)
> @@ -199,7 +217,7 @@ void mte_thread_init_user(void)
>
>  void mte_thread_switch(struct task_struct *next)
>  {
> -       mte_update_sctlr_user(next);
> +       mte_update_sctlr_user_and_gcr_excl(next);
>
>         /*
>          * Check if an async tag exception occurred at EL1.
> @@ -244,7 +262,7 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
>         task->thread.mte_ctrl = mte_ctrl;
>         if (task == current) {
>                 preempt_disable();
> -               mte_update_sctlr_user(task);
> +               mte_update_sctlr_user_and_gcr_excl(task);
>                 update_sctlr_el1(task->thread.sctlr_user);
>                 preempt_enable();
>         }
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index dd874a1ee862..de5f5913374d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -89,7 +89,7 @@ static __always_inline bool kasan_enabled(void)
>         return static_branch_likely(&kasan_flag_enabled);
>  }
>
> -static inline bool kasan_has_integrated_init(void)
> +static inline bool kasan_hw_tags_enabled(void)
>  {
>         return kasan_enabled();
>  }
> @@ -104,7 +104,7 @@ static inline bool kasan_enabled(void)
>         return IS_ENABLED(CONFIG_KASAN);
>  }
>
> -static inline bool kasan_has_integrated_init(void)
> +static inline bool kasan_hw_tags_enabled(void)
>  {
>         return false;
>  }
> @@ -125,6 +125,11 @@ static __always_inline void kasan_free_pages(struct page *page,
>
>  #endif /* CONFIG_KASAN_HW_TAGS */
>
> +static inline bool kasan_has_integrated_init(void)
> +{
> +       return kasan_hw_tags_enabled();
> +}
> +
>  #ifdef CONFIG_KASAN
>
>  struct kasan_cache {
> --
> 2.33.0.259.gc128427fd7-goog
>

Acked-by: Andrey Konovalov <andreyknvl@gmail.com>

for non-arm-specific KASAN parts.

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

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

[Peter Collingbourne]

On Sun, Aug 29, 2021 at 7:10 AM Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> On Fri, Aug 27, 2021 at 5:07 AM Peter Collingbourne <pcc@google.com> wrote:
> >
> > It is not necessary to write to GCR_EL1 on every kernel entry and
> > exit when HW tag-based KASAN is disabled because the kernel will not
> > execute any IRG instructions in that mode. Since accessing GCR_EL1
> > can be expensive on some microarchitectures, avoid doing so by moving
> > the access to task switch when HW tag-based KASAN is disabled.
> >
> > Signed-off-by: Peter Collingbourne <pcc@google.com>
> > ---
> >  arch/arm64/kernel/entry.S | 10 +++++-----
> >  arch/arm64/kernel/mte.c   | 24 +++++++++++++++++++++---
> >  include/linux/kasan.h     |  9 +++++++--
> >  3 files changed, 33 insertions(+), 10 deletions(-)
> >
> > 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());
> >  }
> >
> >  void mte_thread_init_user(void)
> > @@ -199,7 +217,7 @@ void mte_thread_init_user(void)
> >
> >  void mte_thread_switch(struct task_struct *next)
> >  {
> > -       mte_update_sctlr_user(next);
> > +       mte_update_sctlr_user_and_gcr_excl(next);
> >
> >         /*
> >          * Check if an async tag exception occurred at EL1.
> > @@ -244,7 +262,7 @@ long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> >         task->thread.mte_ctrl = mte_ctrl;
> >         if (task == current) {
> >                 preempt_disable();
> > -               mte_update_sctlr_user(task);
> > +               mte_update_sctlr_user_and_gcr_excl(task);
> >                 update_sctlr_el1(task->thread.sctlr_user);
> >                 preempt_enable();
> >         }
> > diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> > index dd874a1ee862..de5f5913374d 100644
> > --- a/include/linux/kasan.h
> > +++ b/include/linux/kasan.h
> > @@ -89,7 +89,7 @@ static __always_inline bool kasan_enabled(void)
> >         return static_branch_likely(&kasan_flag_enabled);
> >  }
> >
> > -static inline bool kasan_has_integrated_init(void)
> > +static inline bool kasan_hw_tags_enabled(void)
> >  {
> >         return kasan_enabled();
> >  }
> > @@ -104,7 +104,7 @@ static inline bool kasan_enabled(void)
> >         return IS_ENABLED(CONFIG_KASAN);
> >  }
> >
> > -static inline bool kasan_has_integrated_init(void)
> > +static inline bool kasan_hw_tags_enabled(void)
> >  {
> >         return false;
> >  }
> > @@ -125,6 +125,11 @@ static __always_inline void kasan_free_pages(struct page *page,
> >
> >  #endif /* CONFIG_KASAN_HW_TAGS */
> >
> > +static inline bool kasan_has_integrated_init(void)
> > +{
> > +       return kasan_hw_tags_enabled();
> > +}
> > +
> >  #ifdef CONFIG_KASAN
> >
> >  struct kasan_cache {
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
>
> Acked-by: Andrey Konovalov <andreyknvl@gmail.com>
>
> for non-arm-specific KASAN parts.

Hi Catalin,

Would you be able to take a look please? Do you think it's possible
that we could get this in before the end of the merge window?

Peter

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

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

[Catalin Marinas]

On Thu, Aug 26, 2021 at 08:07:48PM -0700, Peter Collingbourne wrote:
> It is not necessary to write to GCR_EL1 on every kernel entry and
> exit when HW tag-based KASAN is disabled because the kernel will not
> execute any IRG instructions in that mode. Since accessing GCR_EL1
> can be expensive on some microarchitectures, avoid doing so by moving
> the access to task switch when HW tag-based KASAN is disabled.

I haven't got a chance to look at this patch yet as it was posted pretty
close to the merging window, so it wasn't on my urgent list (not a fix).

What's the use-case exactly? Do you expect production kernels to have
KASAN_HW built in but disabled on the command line? I thought there's an
overhead already from kasan even without MTE.

-- 
Catalin

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

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

[Peter Collingbourne]

On Mon, Sep 6, 2021 at 10:58 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Thu, Aug 26, 2021 at 08:07:48PM -0700, Peter Collingbourne wrote:
> > It is not necessary to write to GCR_EL1 on every kernel entry and
> > exit when HW tag-based KASAN is disabled because the kernel will not
> > execute any IRG instructions in that mode. Since accessing GCR_EL1
> > can be expensive on some microarchitectures, avoid doing so by moving
> > the access to task switch when HW tag-based KASAN is disabled.
>
> I haven't got a chance to look at this patch yet as it was posted pretty
> close to the merging window, so it wasn't on my urgent list (not a fix).
>
> What's the use-case exactly? Do you expect production kernels to have
> KASAN_HW built in but disabled on the command line?

Yes, KASAN_HW_TAGS will be enabled in the GKI, but we would still like
the flexibility of allowing vendors to disable it via the command
line. If they choose to do so, the overhead should be as small as
possible.

> I thought there's an
> overhead already from kasan even without MTE.

I believe that the intent is that KASAN_HW_TAGS on its own (with
kasan=off or on unsupported hardware) should not introduce any
significant overhead.

Peter

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

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

[Catalin Marinas]

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.

-- 
Catalin

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

Re: [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@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@entry=0xffff80001211bd10, is_module=false,
    feature_mask=feature_mask@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@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@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

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

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

[Peter Collingbourne]

On Thu, Aug 26, 2021 at 8:07 PM Peter Collingbourne <pcc@google.com> wrote:
>
> It is not necessary to write to GCR_EL1 on every kernel entry and
> exit when HW tag-based KASAN is disabled because the kernel will not
> execute any IRG instructions in that mode. Since accessing GCR_EL1
> can be expensive on some microarchitectures, avoid doing so by moving
> the access to task switch when HW tag-based KASAN is disabled.

It was discovered that this patch broke booting on non-MTE hardware
when configured with CONFIG_ARM64_MTE=y. Fixed in v2.

Peter

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