[PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Lexi Shao]

In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
error code is store in register int __e(r0). When kasan is
enabled, assigning value to kernel address might trigger kasan check,
which unexpectedly overwrites r0 and causes undefined behavior on arm
kasan images.

One example is failure in do_futex and results in process soft lockup.
Log:
watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
Q:Reg:1151]
...
(__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
(futex_wait_setup) from (futex_wait+0x138/0x394)
(futex_wait) from (do_futex+0x164/0xe40)
(do_futex) from (sys_futex_time32+0x178/0x230)
(sys_futex_time32) from (ret_fast_syscall+0x0/0x50)

The soft lockup happens in function futex_wait_setup. The reason is
function get_futex_value_locked always return EINVAL, thus pc jump
back to retry label and causes looping.

The assembly code of get_futex_value_locked in kernel/futex.c:
...
c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
// "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
c01f6dcc:       e1a00007        mov     r0, r7
c01f6dd0:       e1a05002        mov     r5, r2
c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
c01f6dd8:       e5875000        str     r5, [r7]
// save ret value of __get_user(*dest, from), which is dest address now
c01f6ddc:       e1a05000        mov     r5, r0
...
// checking return value of __get_user failed
c01f6e00:       e3550000        cmp     r5, #0
...
c01f6e0c:       01a00005        moveq   r0, r5
// assign return value to EINVAL
c01f6e10:       13e0000d        mvnne   r0, #13

Return value is the destination address of get_user thus certainly
non-zero, so get_futex_value_locked always return EINVAL.

Fix it by using a tmp vairable to store the error code before the
assignment. This fix has no effects to non-kasan images thanks to compiler
optimization. It only affects cases that overwrite r0 due to kasan check.

This should fix bug discussed in link:
[1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/

Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 arch/arm/include/asm/uaccess.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index a13d90206472..a6eb9af74870 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
 		register unsigned long __l asm("r1") = __limit;		\
 		register int __e asm("r0");				\
 		unsigned int __ua_flags = uaccess_save_and_enable();	\
+		int __tmp_e;						\
 		switch (sizeof(*(__p))) {				\
 		case 1:							\
 			if (sizeof((x)) >= 8)				\
@@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
 			break;						\
 		default: __e = __get_user_bad(); break;			\
 		}							\
+		__tmp_e = __e;						\
 		uaccess_restore(__ua_flags);				\
 		x = (typeof(*(p))) __r2;				\
-		__e;							\
+		__e = __tmp_e;						\
 	})
 
 #define get_user(x, p)							\
-- 
2.12.3


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

Re: [PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Dmitry Osipenko]

25.08.2021 09:46, Lexi Shao пишет:
> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
> error code is store in register int __e(r0). When kasan is
> enabled, assigning value to kernel address might trigger kasan check,
> which unexpectedly overwrites r0 and causes undefined behavior on arm
> kasan images.
> 
> One example is failure in do_futex and results in process soft lockup.
> Log:
> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
> Q:Reg:1151]
> ...
> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
> (futex_wait_setup) from (futex_wait+0x138/0x394)
> (futex_wait) from (do_futex+0x164/0xe40)
> (do_futex) from (sys_futex_time32+0x178/0x230)
> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
> 
> The soft lockup happens in function futex_wait_setup. The reason is
> function get_futex_value_locked always return EINVAL, thus pc jump
> back to retry label and causes looping.
> 
> The assembly code of get_futex_value_locked in kernel/futex.c:
> ...
> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
> c01f6dcc:       e1a00007        mov     r0, r7
> c01f6dd0:       e1a05002        mov     r5, r2
> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
> c01f6dd8:       e5875000        str     r5, [r7]
> // save ret value of __get_user(*dest, from), which is dest address now
> c01f6ddc:       e1a05000        mov     r5, r0
> ...
> // checking return value of __get_user failed
> c01f6e00:       e3550000        cmp     r5, #0
> ...
> c01f6e0c:       01a00005        moveq   r0, r5
> // assign return value to EINVAL
> c01f6e10:       13e0000d        mvnne   r0, #13
> 
> Return value is the destination address of get_user thus certainly
> non-zero, so get_futex_value_locked always return EINVAL.
> 
> Fix it by using a tmp vairable to store the error code before the
> assignment. This fix has no effects to non-kasan images thanks to compiler
> optimization. It only affects cases that overwrite r0 due to kasan check.
> 
> This should fix bug discussed in link:
> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
> 
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> ---
>  arch/arm/include/asm/uaccess.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index a13d90206472..a6eb9af74870 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>  		register unsigned long __l asm("r1") = __limit;		\
>  		register int __e asm("r0");				\
>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
> +		int __tmp_e;						\
>  		switch (sizeof(*(__p))) {				\
>  		case 1:							\
>  			if (sizeof((x)) >= 8)				\
> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>  			break;						\
>  		default: __e = __get_user_bad(); break;			\
>  		}							\
> +		__tmp_e = __e;						\
>  		uaccess_restore(__ua_flags);				\
>  		x = (typeof(*(p))) __r2;				\
> -		__e;							\
> +		__e = __tmp_e;						\
>  	})
>  
>  #define get_user(x, p)							\
> 

I successfully loaded KDE Plasma with this fix, which was impossible
previously. Thank you!

Is it guaranteed that r2 register won't be clobbered as well?

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

Re: [PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Kefeng Wang]

On 2021/8/25 14:46, Lexi Shao wrote:
> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
> error code is store in register int __e(r0). When kasan is
> enabled, assigning value to kernel address might trigger kasan check,
> which unexpectedly overwrites r0 and causes undefined behavior on arm
> kasan images.
>
> One example is failure in do_futex and results in process soft lockup.
> Log:
> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
> Q:Reg:1151]
> ...
> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
> (futex_wait_setup) from (futex_wait+0x138/0x394)
> (futex_wait) from (do_futex+0x164/0xe40)
> (do_futex) from (sys_futex_time32+0x178/0x230)
> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
>
> The soft lockup happens in function futex_wait_setup. The reason is
> function get_futex_value_locked always return EINVAL, thus pc jump
> back to retry label and causes looping.
>
> The assembly code of get_futex_value_locked in kernel/futex.c:
> ...
> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
> c01f6dcc:       e1a00007        mov     r0, r7
> c01f6dd0:       e1a05002        mov     r5, r2
> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
> c01f6dd8:       e5875000        str     r5, [r7]
> // save ret value of __get_user(*dest, from), which is dest address now
> c01f6ddc:       e1a05000        mov     r5, r0
> ...
> // checking return value of __get_user failed
> c01f6e00:       e3550000        cmp     r5, #0
> ...
> c01f6e0c:       01a00005        moveq   r0, r5
> // assign return value to EINVAL
> c01f6e10:       13e0000d        mvnne   r0, #13
>
> Return value is the destination address of get_user thus certainly
> non-zero, so get_futex_value_locked always return EINVAL.
>
> Fix it by using a tmp vairable to store the error code before the
> assignment. This fix has no effects to non-kasan images thanks to compiler
> optimization. It only affects cases that overwrite r0 due to kasan check.
>
> This should fix bug discussed in link:
> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
>
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> ---
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>

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

Re: [PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Russell King (Oracle)]

On Wed, Aug 25, 2021 at 02:46:50PM +0800, Lexi Shao wrote:
> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
> error code is store in register int __e(r0). When kasan is
> enabled, assigning value to kernel address might trigger kasan check,
> which unexpectedly overwrites r0 and causes undefined behavior on arm
> kasan images.
> 
> One example is failure in do_futex and results in process soft lockup.
> Log:
> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
> Q:Reg:1151]
> ...
> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
> (futex_wait_setup) from (futex_wait+0x138/0x394)
> (futex_wait) from (do_futex+0x164/0xe40)
> (do_futex) from (sys_futex_time32+0x178/0x230)
> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
> 
> The soft lockup happens in function futex_wait_setup. The reason is
> function get_futex_value_locked always return EINVAL, thus pc jump
> back to retry label and causes looping.
> 
> The assembly code of get_futex_value_locked in kernel/futex.c:
> ...
> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
> c01f6dcc:       e1a00007        mov     r0, r7
> c01f6dd0:       e1a05002        mov     r5, r2
> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
> c01f6dd8:       e5875000        str     r5, [r7]
> // save ret value of __get_user(*dest, from), which is dest address now
> c01f6ddc:       e1a05000        mov     r5, r0
> ...
> // checking return value of __get_user failed
> c01f6e00:       e3550000        cmp     r5, #0
> ...
> c01f6e0c:       01a00005        moveq   r0, r5
> // assign return value to EINVAL
> c01f6e10:       13e0000d        mvnne   r0, #13
> 
> Return value is the destination address of get_user thus certainly
> non-zero, so get_futex_value_locked always return EINVAL.

This description doesn't actually make it clear why this is failing in
this case, until one looks at get_futex_value_locked() and realises
that what is actually going on here is:

	*dest = (typeof(*(p))) __r2;

which is why we end up with the store there.

> Fix it by using a tmp vairable to store the error code before the
> assignment. This fix has no effects to non-kasan images thanks to compiler
> optimization. It only affects cases that overwrite r0 due to kasan check.
> 
> This should fix bug discussed in link:
> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
> 
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> ---
>  arch/arm/include/asm/uaccess.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index a13d90206472..a6eb9af74870 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>  		register unsigned long __l asm("r1") = __limit;		\
>  		register int __e asm("r0");				\
>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
> +		int __tmp_e;						\
>  		switch (sizeof(*(__p))) {				\
>  		case 1:							\
>  			if (sizeof((x)) >= 8)				\
> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>  			break;						\
>  		default: __e = __get_user_bad(); break;			\
>  		}							\
> +		__tmp_e = __e;						\
>  		uaccess_restore(__ua_flags);				\
>  		x = (typeof(*(p))) __r2;				\
> -		__e;							\
> +		__e = __tmp_e;						\

There is no need to re-assign __tmp_e back to __e - you can just return
__tmp_e here.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

Re: [PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Lexi Shao]

>On Wed, Aug 25, 2021 at 02:46:50PM +0800, Lexi Shao wrote:
>> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
>> error code is store in register int __e(r0). When kasan is
>> enabled, assigning value to kernel address might trigger kasan check,
>> which unexpectedly overwrites r0 and causes undefined behavior on arm
>> kasan images.
>> 
>> One example is failure in do_futex and results in process soft lockup.
>> Log:
>> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
>> Q:Reg:1151]
>> ...
>> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
>> (futex_wait_setup) from (futex_wait+0x138/0x394)
>> (futex_wait) from (do_futex+0x164/0xe40)
>> (do_futex) from (sys_futex_time32+0x178/0x230)
>> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
>> 
>> The soft lockup happens in function futex_wait_setup. The reason is
>> function get_futex_value_locked always return EINVAL, thus pc jump
>> back to retry label and causes looping.
>> 
>> The assembly code of get_futex_value_locked in kernel/futex.c:
>> ...
>> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
>> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
>> c01f6dcc:       e1a00007        mov     r0, r7
>> c01f6dd0:       e1a05002        mov     r5, r2
>> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
>> c01f6dd8:       e5875000        str     r5, [r7]
>> // save ret value of __get_user(*dest, from), which is dest address now
>> c01f6ddc:       e1a05000        mov     r5, r0
>> ...
>> // checking return value of __get_user failed
>> c01f6e00:       e3550000        cmp     r5, #0
>> ...
>> c01f6e0c:       01a00005        moveq   r0, r5
>> // assign return value to EINVAL
>> c01f6e10:       13e0000d        mvnne   r0, #13
>> 
>> Return value is the destination address of get_user thus certainly
>> non-zero, so get_futex_value_locked always return EINVAL.
>
>This description doesn't actually make it clear why this is failing in
>this case, until one looks at get_futex_value_locked() and realises
>that what is actually going on here is:
>
>	*dest = (typeof(*(p))) __r2;
>
>which is why we end up with the store there.
>
>> Fix it by using a tmp vairable to store the error code before the
>> assignment. This fix has no effects to non-kasan images thanks to compiler
>> optimization. It only affects cases that overwrite r0 due to kasan check.
>> 
>> This should fix bug discussed in link:
>> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
>> 
>> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>> ---
>>  arch/arm/include/asm/uaccess.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
>> index a13d90206472..a6eb9af74870 100644
>> --- a/arch/arm/include/asm/uaccess.h
>> +++ b/arch/arm/include/asm/uaccess.h
>> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>>  		register unsigned long __l asm("r1") = __limit;		\
>>  		register int __e asm("r0");				\
>>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
>> +		int __tmp_e;						\
>>  		switch (sizeof(*(__p))) {				\
>>  		case 1:							\
>>  			if (sizeof((x)) >= 8)				\
>> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>>  			break;						\
>>  		default: __e = __get_user_bad(); break;			\
>>  		}							\
>> +		__tmp_e = __e;						\
>>  		uaccess_restore(__ua_flags);				\
>>  		x = (typeof(*(p))) __r2;				\
>> -		__e;							\
>> +		__e = __tmp_e;						\
>
>There is no need to re-assign __tmp_e back to __e - you can just return
>__tmp_e here.

Yes you are right, will send the updated patch in the next email.

Lexi


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

[PATCH v2] ARM: kasan: Fix __get_user_check failure with kasan

[Lexi Shao]

In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
error code is store in register int __e(r0). When kasan is
enabled, assigning value to kernel address might trigger kasan check,
which unexpectedly overwrites r0 and causes undefined behavior on arm
kasan images.

One example is failure in do_futex and results in process soft lockup.
Log:
watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
Q:Reg:1151]
...
(__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
(futex_wait_setup) from (futex_wait+0x138/0x394)
(futex_wait) from (do_futex+0x164/0xe40)
(do_futex) from (sys_futex_time32+0x178/0x230)
(sys_futex_time32) from (ret_fast_syscall+0x0/0x50)

The soft lockup happens in function futex_wait_setup. The reason is
function get_futex_value_locked always return EINVAL, thus pc jump
back to retry label and causes looping.

This line in function get_futex_value_locked
	ret = __get_user(*dest, from);
is expanded to
	*dest = (typeof(*(p))) __r2; ,
in macro __get_user_check. Writing to pointer dest triggers kasan check
and overwrites the return value of __get_user_x function.
The assembly code of get_futex_value_locked in kernel/futex.c:
...
c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
// "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
c01f6dcc:       e1a00007        mov     r0, r7
c01f6dd0:       e1a05002        mov     r5, r2
c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
c01f6dd8:       e5875000        str     r5, [r7]
// save ret value of __get_user(*dest, from), which is dest address now
c01f6ddc:       e1a05000        mov     r5, r0
...
// checking return value of __get_user failed
c01f6e00:       e3550000        cmp     r5, #0
...
c01f6e0c:       01a00005        moveq   r0, r5
// assign return value to EINVAL
c01f6e10:       13e0000d        mvnne   r0, #13

Return value is the destination address of get_user thus certainly
non-zero, so get_futex_value_locked always return EINVAL.

Fix it by using a tmp vairable to store the error code before the
assignment. This fix has no effects to non-kasan images thanks to compiler
optimization. It only affects cases that overwrite r0 due to kasan check.

This should fix bug discussed in link:
[1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/

Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
Signed-off-by: Lexi Shao <shaolexi@huawei.com>
---
 arch/arm/include/asm/uaccess.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index a13d90206472..d9db752c51fe 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
 		register unsigned long __l asm("r1") = __limit;		\
 		register int __e asm("r0");				\
 		unsigned int __ua_flags = uaccess_save_and_enable();	\
+		int __tmp_e;						\
 		switch (sizeof(*(__p))) {				\
 		case 1:							\
 			if (sizeof((x)) >= 8)				\
@@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
 			break;						\
 		default: __e = __get_user_bad(); break;			\
 		}							\
+		__tmp_e = __e;						\
 		uaccess_restore(__ua_flags);				\
 		x = (typeof(*(p))) __r2;				\
-		__e;							\
+		__tmp_e;						\
 	})
 
 #define get_user(x, p)							\
-- 
2.12.3


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

Re: [PATCH] ARM: kasan: Fix __get_user_check failure with kasan

[Lexi Shao]

>25.08.2021 09:46, Lexi Shao пишет:
>> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
>> error code is store in register int __e(r0). When kasan is
>> enabled, assigning value to kernel address might trigger kasan check,
>> which unexpectedly overwrites r0 and causes undefined behavior on arm
>> kasan images.
>> 
>> One example is failure in do_futex and results in process soft lockup.
>> Log:
>> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
>> Q:Reg:1151]
>> ...
>> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
>> (futex_wait_setup) from (futex_wait+0x138/0x394)
>> (futex_wait) from (do_futex+0x164/0xe40)
>> (do_futex) from (sys_futex_time32+0x178/0x230)
>> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
>> 
>> The soft lockup happens in function futex_wait_setup. The reason is
>> function get_futex_value_locked always return EINVAL, thus pc jump
>> back to retry label and causes looping.
>> 
>> The assembly code of get_futex_value_locked in kernel/futex.c:
>> ...
>> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
>> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
>> c01f6dcc:       e1a00007        mov     r0, r7
>> c01f6dd0:       e1a05002        mov     r5, r2
>> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
>> c01f6dd8:       e5875000        str     r5, [r7]
>> // save ret value of __get_user(*dest, from), which is dest address now
>> c01f6ddc:       e1a05000        mov     r5, r0
>> ...
>> // checking return value of __get_user failed
>> c01f6e00:       e3550000        cmp     r5, #0
>> ...
>> c01f6e0c:       01a00005        moveq   r0, r5
>> // assign return value to EINVAL
>> c01f6e10:       13e0000d        mvnne   r0, #13
>> 
>> Return value is the destination address of get_user thus certainly
>> non-zero, so get_futex_value_locked always return EINVAL.
>> 
>> Fix it by using a tmp vairable to store the error code before the
>> assignment. This fix has no effects to non-kasan images thanks to compiler
>> optimization. It only affects cases that overwrite r0 due to kasan check.
>> 
>> This should fix bug discussed in link:
>> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
>> 
>> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>> ---
>>  arch/arm/include/asm/uaccess.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
>> index a13d90206472..a6eb9af74870 100644
>> --- a/arch/arm/include/asm/uaccess.h
>> +++ b/arch/arm/include/asm/uaccess.h
>> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>>  		register unsigned long __l asm("r1") = __limit;		\
>>  		register int __e asm("r0");				\
>>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
>> +		int __tmp_e;						\
>>  		switch (sizeof(*(__p))) {				\
>>  		case 1:							\
>>  			if (sizeof((x)) >= 8)				\
>> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>>  			break;						\
>>  		default: __e = __get_user_bad(); break;			\
>>  		}							\
>> +		__tmp_e = __e;						\
>>  		uaccess_restore(__ua_flags);				\
>>  		x = (typeof(*(p))) __r2;				\
>> -		__e;							\
>> +		__e = __tmp_e;						\
>>  	})
>>  
>>  #define get_user(x, p)							\
>> 
>
>I successfully loaded KDE Plasma with this fix, which was impossible
>previously. Thank you!
>
>Is it guaranteed that r2 register won't be clobbered as well?

Value of r2 is saved before calling __asan_store4:

c01f6dbc:       e5951008        ldr     r1, [r5, #8]
c01f6dc0:       e1a00008        mov     r0, r8
c01f6dc4:       e2411001        sub     r1, r1, #1
c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
c01f6dcc:       e1a00007        mov     r0, r7
// move value of r2 to r5
c01f6dd0:       e1a05002        mov     r5, r2
c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
// the assignment, saving r5(orig r2) to dest
c01f6dd8:       e5875000        str     r5, [r7]
c01f6ddc:       e1a05000        mov     r5, r0

I can't explain it clearly why the compiler save r2 but not r0 though...


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

Re: [PATCH v2] ARM: kasan: Fix __get_user_check failure with kasan

[Dmitry Osipenko]

28.08.2021 05:25, Lexi Shao пишет:
> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
> error code is store in register int __e(r0). When kasan is
> enabled, assigning value to kernel address might trigger kasan check,
> which unexpectedly overwrites r0 and causes undefined behavior on arm
> kasan images.
> 
> One example is failure in do_futex and results in process soft lockup.
> Log:
> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
> Q:Reg:1151]
> ...
> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
> (futex_wait_setup) from (futex_wait+0x138/0x394)
> (futex_wait) from (do_futex+0x164/0xe40)
> (do_futex) from (sys_futex_time32+0x178/0x230)
> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
> 
> The soft lockup happens in function futex_wait_setup. The reason is
> function get_futex_value_locked always return EINVAL, thus pc jump
> back to retry label and causes looping.
> 
> This line in function get_futex_value_locked
> 	ret = __get_user(*dest, from);
> is expanded to
> 	*dest = (typeof(*(p))) __r2; ,
> in macro __get_user_check. Writing to pointer dest triggers kasan check
> and overwrites the return value of __get_user_x function.
> The assembly code of get_futex_value_locked in kernel/futex.c:
> ...
> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
> c01f6dcc:       e1a00007        mov     r0, r7
> c01f6dd0:       e1a05002        mov     r5, r2
> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
> c01f6dd8:       e5875000        str     r5, [r7]
> // save ret value of __get_user(*dest, from), which is dest address now
> c01f6ddc:       e1a05000        mov     r5, r0
> ...
> // checking return value of __get_user failed
> c01f6e00:       e3550000        cmp     r5, #0
> ...
> c01f6e0c:       01a00005        moveq   r0, r5
> // assign return value to EINVAL
> c01f6e10:       13e0000d        mvnne   r0, #13
> 
> Return value is the destination address of get_user thus certainly
> non-zero, so get_futex_value_locked always return EINVAL.
> 
> Fix it by using a tmp vairable to store the error code before the
> assignment. This fix has no effects to non-kasan images thanks to compiler
> optimization. It only affects cases that overwrite r0 due to kasan check.
> 
> This should fix bug discussed in link:
> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
> 
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
> ---
>  arch/arm/include/asm/uaccess.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index a13d90206472..d9db752c51fe 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>  		register unsigned long __l asm("r1") = __limit;		\
>  		register int __e asm("r0");				\
>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
> +		int __tmp_e;						\
>  		switch (sizeof(*(__p))) {				\
>  		case 1:							\
>  			if (sizeof((x)) >= 8)				\
> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>  			break;						\
>  		default: __e = __get_user_bad(); break;			\
>  		}							\
> +		__tmp_e = __e;						\
>  		uaccess_restore(__ua_flags);				\
>  		x = (typeof(*(p))) __r2;				\
> -		__e;							\
> +		__tmp_e;						\
>  	})
>  
>  #define get_user(x, p)							\
> 

Tested-by: Dmitry Osipenko <digetx@gmail.com> # Tegra

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

Re: [PATCH v2] ARM: kasan: Fix __get_user_check failure with kasan

[Kefeng Wang]

On 2021/8/28 10:25, Lexi Shao wrote:
> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
> error code is store in register int __e(r0). When kasan is
> enabled, assigning value to kernel address might trigger kasan check,
> which unexpectedly overwrites r0 and causes undefined behavior on arm
> kasan images.
>
> One example is failure in do_futex and results in process soft lockup.
> Log:
> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
> Q:Reg:1151]
> ...
> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
> (futex_wait_setup) from (futex_wait+0x138/0x394)
> (futex_wait) from (do_futex+0x164/0xe40)
> (do_futex) from (sys_futex_time32+0x178/0x230)
> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
>
> The soft lockup happens in function futex_wait_setup. The reason is
> function get_futex_value_locked always return EINVAL, thus pc jump
> back to retry label and causes looping.
>
> This line in function get_futex_value_locked
> 	ret = __get_user(*dest, from);
> is expanded to
> 	*dest = (typeof(*(p))) __r2; ,
> in macro __get_user_check. Writing to pointer dest triggers kasan check
> and overwrites the return value of __get_user_x function.
> The assembly code of get_futex_value_locked in kernel/futex.c:
> ...
> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
> c01f6dcc:       e1a00007        mov     r0, r7
> c01f6dd0:       e1a05002        mov     r5, r2
> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
> c01f6dd8:       e5875000        str     r5, [r7]
> // save ret value of __get_user(*dest, from), which is dest address now
> c01f6ddc:       e1a05000        mov     r5, r0
> ...
> // checking return value of __get_user failed
> c01f6e00:       e3550000        cmp     r5, #0
> ...
> c01f6e0c:       01a00005        moveq   r0, r5
> // assign return value to EINVAL
> c01f6e10:       13e0000d        mvnne   r0, #13
>
> Return value is the destination address of get_user thus certainly
> non-zero, so get_futex_value_locked always return EINVAL.
>
> Fix it by using a tmp vairable to store the error code before the
> assignment. This fix has no effects to non-kasan images thanks to compiler
> optimization. It only affects cases that overwrite r0 due to kasan check.
>
> This should fix bug discussed in link:
> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
>
> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
> Signed-off-by: Lexi Shao <shaolexi@huawei.com>

Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>


> ---
>   arch/arm/include/asm/uaccess.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index a13d90206472..d9db752c51fe 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>   		register unsigned long __l asm("r1") = __limit;		\
>   		register int __e asm("r0");				\
>   		unsigned int __ua_flags = uaccess_save_and_enable();	\
> +		int __tmp_e;						\
>   		switch (sizeof(*(__p))) {				\
>   		case 1:							\
>   			if (sizeof((x)) >= 8)				\
> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>   			break;						\
>   		default: __e = __get_user_bad(); break;			\
>   		}							\
> +		__tmp_e = __e;						\
>   		uaccess_restore(__ua_flags);				\
>   		x = (typeof(*(p))) __r2;				\
> -		__e;							\
> +		__tmp_e;						\
>   	})
>   
>   #define get_user(x, p)							\

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

Re: [PATCH v2] ARM: kasan: Fix __get_user_check failure with kasan

[Dmitry Osipenko]

31.08.2021 12:51, Dmitry Osipenko пишет:
> 28.08.2021 05:25, Lexi Shao пишет:
>> In macro __get_user_check defined in arch/arm/include/asm/uaccess.h,
>> error code is store in register int __e(r0). When kasan is
>> enabled, assigning value to kernel address might trigger kasan check,
>> which unexpectedly overwrites r0 and causes undefined behavior on arm
>> kasan images.
>>
>> One example is failure in do_futex and results in process soft lockup.
>> Log:
>> watchdog: BUG: soft lockup - CPU#0 stuck for 62946ms! [rs:main
>> Q:Reg:1151]
>> ...
>> (__asan_store4) from (futex_wait_setup+0xf8/0x2b4)
>> (futex_wait_setup) from (futex_wait+0x138/0x394)
>> (futex_wait) from (do_futex+0x164/0xe40)
>> (do_futex) from (sys_futex_time32+0x178/0x230)
>> (sys_futex_time32) from (ret_fast_syscall+0x0/0x50)
>>
>> The soft lockup happens in function futex_wait_setup. The reason is
>> function get_futex_value_locked always return EINVAL, thus pc jump
>> back to retry label and causes looping.
>>
>> This line in function get_futex_value_locked
>> 	ret = __get_user(*dest, from);
>> is expanded to
>> 	*dest = (typeof(*(p))) __r2; ,
>> in macro __get_user_check. Writing to pointer dest triggers kasan check
>> and overwrites the return value of __get_user_x function.
>> The assembly code of get_futex_value_locked in kernel/futex.c:
>> ...
>> c01f6dc8:       eb0b020e        bl      c04b7608 <__get_user_4>
>> // "x = (typeof(*(p))) __r2;" triggers kasan check and r0 is overwritten
>> c01f6dcc:       e1a00007        mov     r0, r7
>> c01f6dd0:       e1a05002        mov     r5, r2
>> c01f6dd4:       eb04f1e6        bl      c0333574 <__asan_store4>
>> c01f6dd8:       e5875000        str     r5, [r7]
>> // save ret value of __get_user(*dest, from), which is dest address now
>> c01f6ddc:       e1a05000        mov     r5, r0
>> ...
>> // checking return value of __get_user failed
>> c01f6e00:       e3550000        cmp     r5, #0
>> ...
>> c01f6e0c:       01a00005        moveq   r0, r5
>> // assign return value to EINVAL
>> c01f6e10:       13e0000d        mvnne   r0, #13
>>
>> Return value is the destination address of get_user thus certainly
>> non-zero, so get_futex_value_locked always return EINVAL.
>>
>> Fix it by using a tmp vairable to store the error code before the
>> assignment. This fix has no effects to non-kasan images thanks to compiler
>> optimization. It only affects cases that overwrite r0 due to kasan check.
>>
>> This should fix bug discussed in link:
>> [1] https://lore.kernel.org/linux-arm-kernel/0ef7c2a5-5d8b-c5e0-63fa-31693fd4495c@gmail.com/
>>
>> Fixes: 421015713b30 ("ARM: 9017/2: Enable KASan for ARM")
>> Signed-off-by: Lexi Shao <shaolexi@huawei.com>
>> ---
>>  arch/arm/include/asm/uaccess.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
>> index a13d90206472..d9db752c51fe 100644
>> --- a/arch/arm/include/asm/uaccess.h
>> +++ b/arch/arm/include/asm/uaccess.h
>> @@ -200,6 +200,7 @@ extern int __get_user_64t_4(void *);
>>  		register unsigned long __l asm("r1") = __limit;		\
>>  		register int __e asm("r0");				\
>>  		unsigned int __ua_flags = uaccess_save_and_enable();	\
>> +		int __tmp_e;						\
>>  		switch (sizeof(*(__p))) {				\
>>  		case 1:							\
>>  			if (sizeof((x)) >= 8)				\
>> @@ -227,9 +228,10 @@ extern int __get_user_64t_4(void *);
>>  			break;						\
>>  		default: __e = __get_user_bad(); break;			\
>>  		}							\
>> +		__tmp_e = __e;						\
>>  		uaccess_restore(__ua_flags);				\
>>  		x = (typeof(*(p))) __r2;				\
>> -		__e;							\
>> +		__tmp_e;						\
>>  	})
>>  
>>  #define get_user(x, p)							\
>>
> 
> Tested-by: Dmitry Osipenko <digetx@gmail.com> # Tegra
> 

Note that you may need to submit this patch to
https://www.armlinux.org.uk/developer/patches/ to speed up the applying.

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