Re: [PATCH 5.10 01/77] ARM: 9132/1: Fix __get_user_check failure with ARM KASAN images

From: Florian Fainelli <f.fainelli@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org, Lexi Shao <shaolexi@huawei.com>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
Subject: Re: [PATCH 5.10 01/77] ARM: 9132/1: Fix __get_user_check failure with ARM KASAN images
Date: Fri, 12 Nov 2021 14:56:47 -0800	[thread overview]
Message-ID: <5ea7d722-3f95-6c26-edb2-4b6a82fdbb38@gmail.com> (raw)
In-Reply-To: <20211101082511.523745713@linuxfoundation.org>

On 11/1/21 2:16 AM, Greg Kroah-Hartman wrote:
> From: Lexi Shao <shaolexi@huawei.com>
> 
> commit df909df0770779f1a5560c2bb641a2809655ef28 upstream.
> 
> ARM: kasan: Fix __get_user_check failure with kasan
> 
> 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")

421015713b30 ("ARM: 9017/2: Enable KASan for ARM") is only in 5.11 and
beyond, do we want to take this patch still? I already tested the whole
v5.10.77-rc1 and did not have problems.
-- 
Florian