* [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
@ 2021-08-27 3:07 Peter Collingbourne
2021-08-29 14:10 ` Andrey Konovalov
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-08-27 3:07 UTC (permalink / raw)
To: Catalin Marinas, Vincenzo Frascino, Will Deacon, Andrey Konovalov
Cc: Peter Collingbourne, Evgenii Stepanov, linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-08-27 3:07 [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled Peter Collingbourne
@ 2021-08-29 14:10 ` Andrey Konovalov
2021-09-03 22:12 ` Peter Collingbourne
2021-09-06 17:58 ` Catalin Marinas
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Andrey Konovalov @ 2021-08-29 14:10 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon,
Evgenii Stepanov, Linux ARM
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-08-29 14:10 ` Andrey Konovalov
@ 2021-09-03 22:12 ` Peter Collingbourne
0 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-09-03 22:12 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Catalin Marinas, Vincenzo Frascino, Will Deacon,
Evgenii Stepanov, Linux ARM
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-08-27 3:07 [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled Peter Collingbourne
2021-08-29 14:10 ` Andrey Konovalov
@ 2021-09-06 17:58 ` Catalin Marinas
2021-09-07 23:20 ` Peter Collingbourne
2021-09-09 17:39 ` Catalin Marinas
2021-09-14 21:54 ` Peter Collingbourne
3 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2021-09-06 17:58 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Evgenii Stepanov, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-09-06 17:58 ` Catalin Marinas
@ 2021-09-07 23:20 ` Peter Collingbourne
0 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-09-07 23:20 UTC (permalink / raw)
To: Catalin Marinas
Cc: Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Evgenii Stepanov, Linux ARM
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-08-27 3:07 [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled Peter Collingbourne
2021-08-29 14:10 ` Andrey Konovalov
2021-09-06 17:58 ` Catalin Marinas
@ 2021-09-09 17:39 ` Catalin Marinas
2021-09-10 2:12 ` Peter Collingbourne
2021-09-14 21:54 ` Peter Collingbourne
3 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2021-09-09 17:39 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Evgenii Stepanov, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-09-09 17:39 ` Catalin Marinas
@ 2021-09-10 2:12 ` Peter Collingbourne
0 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-09-10 2:12 UTC (permalink / raw)
To: Catalin Marinas
Cc: Vincenzo Frascino, Will Deacon, Andrey Konovalov,
Evgenii Stepanov, Linux ARM
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled
2021-08-27 3:07 [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled Peter Collingbourne
` (2 preceding siblings ...)
2021-09-09 17:39 ` Catalin Marinas
@ 2021-09-14 21:54 ` Peter Collingbourne
3 siblings, 0 replies; 8+ messages in thread
From: Peter Collingbourne @ 2021-09-14 21:54 UTC (permalink / raw)
To: Catalin Marinas, Vincenzo Frascino, Will Deacon, Andrey Konovalov
Cc: Evgenii Stepanov, Linux ARM
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-14 21:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 3:07 [PATCH] arm64: kasan: mte: move GCR_EL1 switch to task switch when KASAN disabled Peter Collingbourne
2021-08-29 14:10 ` Andrey Konovalov
2021-09-03 22:12 ` Peter Collingbourne
2021-09-06 17:58 ` Catalin Marinas
2021-09-07 23:20 ` Peter Collingbourne
2021-09-09 17:39 ` Catalin Marinas
2021-09-10 2:12 ` Peter Collingbourne
2021-09-14 21:54 ` Peter Collingbourne
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.