diff mbox series

[v2,2/2] ARM: entry: Do a dummy read from VMAP shadow

Message ID 20241016-arm-kasan-vmalloc-crash-v2-2-0a52fd086eef@linaro.org (mailing list archive)
State New, archived
Headers show
Series Fix KASAN crash when using KASAN_VMALLOC | expand

Commit Message

Linus Walleij Oct. 16, 2024, 7:15 p.m. UTC
When switching task, in addition to a dummy read from the new
VMAP stack, also do a dummy read from the VMAP stack's
corresponding KASAN shadow memory to sync things up in
the new MM context.

Cc: stable@vger.kernel.org
Fixes: a1c510d0adc6 ("ARM: implement support for vmap'ed stacks")
Link: https://lore.kernel.org/linux-arm-kernel/a1a1d062-f3a2-4d05-9836-3b098de9db6d@foss.st.com/
Reported-by: Clement LE GOFFIC <clement.legoffic@foss.st.com>
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/kernel/entry-armv.S | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Clément Le Goffic Oct. 17, 2024, 10:17 a.m. UTC | #1
On 10/16/24 21:15, Linus Walleij wrote:
> When switching task, in addition to a dummy read from the new
> VMAP stack, also do a dummy read from the VMAP stack's
> corresponding KASAN shadow memory to sync things up in
> the new MM context.
> 
> Cc: stable@vger.kernel.org
> Fixes: a1c510d0adc6 ("ARM: implement support for vmap'ed stacks")
> Link: https://lore.kernel.org/linux-arm-kernel/a1a1d062-f3a2-4d05-9836-3b098de9db6d@foss.st.com/
> Reported-by: Clement LE GOFFIC <clement.legoffic@foss.st.com>
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   arch/arm/kernel/entry-armv.S | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 1dfae1af8e31..12a4040a04ff 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -25,6 +25,7 @@
>   #include <asm/tls.h>
>   #include <asm/system_info.h>
>   #include <asm/uaccess-asm.h>
> +#include <asm/kasan_def.h>
>   
>   #include "entry-header.S"
>   #include <asm/probes.h>
> @@ -561,6 +562,13 @@ ENTRY(__switch_to)
>   	@ entries covering the vmalloc region.
>   	@
>   	ldr	r2, [ip]
> +#ifdef CONFIG_KASAN_VMALLOC
> +	@ Also dummy read from the KASAN shadow memory for the new stack if we
> +	@ are using KASAN
> +	mov_l	r2, KASAN_SHADOW_OFFSET
> +	add	r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT

Hello Linus,

While ARM TRM says that if Rd is the same of Rn then it can be omitted, 
such syntax causes error on my build.
Looking around for such syntax in the kernel, this line should be :
add	r2, r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT

Regards,

Clement
Linus Walleij Oct. 17, 2024, 12:55 p.m. UTC | #2
On Thu, Oct 17, 2024 at 12:20 PM Clement LE GOFFIC
<clement.legoffic@foss.st.com> wrote:

> > +     add     r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT
(...)
> While ARM TRM says that if Rd is the same of Rn then it can be omitted,
> such syntax causes error on my build.
> Looking around for such syntax in the kernel, this line should be :
> add     r2, r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT

Yeah clearly my compilers allowed it :/

I changed it to the archaic version, will repost as v3.

Please test at your convenience!

Yours,
Linus Walleij
Linus Walleij Oct. 17, 2024, 12:57 p.m. UTC | #3
On Thu, Oct 17, 2024 at 2:55 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Oct 17, 2024 at 12:20 PM Clement LE GOFFIC
> <clement.legoffic@foss.st.com> wrote:
>
> > > +     add     r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT
> (...)
> > While ARM TRM says that if Rd is the same of Rn then it can be omitted,
> > such syntax causes error on my build.
> > Looking around for such syntax in the kernel, this line should be :
> > add     r2, r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT
>
> Yeah clearly my compilers allowed it :/
>
> I changed it to the archaic version, will repost as v3.

I think I meant the canonical version.. anglo-saxon is
sometimes not my strong card.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 1dfae1af8e31..12a4040a04ff 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -25,6 +25,7 @@ 
 #include <asm/tls.h>
 #include <asm/system_info.h>
 #include <asm/uaccess-asm.h>
+#include <asm/kasan_def.h>
 
 #include "entry-header.S"
 #include <asm/probes.h>
@@ -561,6 +562,13 @@  ENTRY(__switch_to)
 	@ entries covering the vmalloc region.
 	@
 	ldr	r2, [ip]
+#ifdef CONFIG_KASAN_VMALLOC
+	@ Also dummy read from the KASAN shadow memory for the new stack if we
+	@ are using KASAN
+	mov_l	r2, KASAN_SHADOW_OFFSET
+	add	r2, ip, lsr #KASAN_SHADOW_SCALE_SHIFT
+	ldr	r2, [r2]
+#endif
 #endif
 
 	@ When CONFIG_THREAD_INFO_IN_TASK=n, the update of SP itself is what