Re: [PATCH] riscv: __asm_copy_to-from_user: fix out of boundary memory copy

From: Akira Tsukamoto <akira.tsukamoto@gmail.com>
To: Qiu Wenbo <qiuwenbo@kylinos.com.cn>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>
Cc: linux-riscv@lists.infradead.org
Subject: Re: [PATCH] riscv: __asm_copy_to-from_user: fix out of boundary memory copy
Date: Sun, 18 Jul 2021 11:05:24 +0900	[thread overview]
Message-ID: <7d492508-728c-2a6d-f3f8-79c519118fe2@gmail.com> (raw)
In-Reply-To: <20210717161213.91892-1-qiuwenbo@kylinos.com.cn>

On 7/18/2021 1:12 AM, Qiu Wenbo wrote:
> The __asm_copy_to-from_user function will copy extra bytes beyond the
> boundary when two conditions hold:
> 
> 1. (src - dst) & (SZREG-1) == 0
> 2. 8*SZREG <= size < -src & (SZREG-1) + 8*SZREG
> 
> The first condition makes the function enter the unrolled word copy code
> path. And the second condition makes the function believe that there is
> enough bytes to do one iteration of 8*SZREG byte copy. That is not true
> since the available bytes is reduced by -src & (SZREG-1) byte to make
> both src and dst aligned to SZREG.

Thanks for analyzing the bug.

>  	li	a3, 8*SZREG /* size must be larger than size in word_copy */

Changing the 8*SZREG to 9*SZREG as bellow

li	a3, 9*SZREG

would fix it but since it is going to respin the patch
I would like to add the word_copy when the size is in between 2*SZREG
9*SZREG as Palmer have mentioned.

Akira

> 
> This behavior causes serious issue with exec system call both on RV64
> and RV32. The passed-in command line parameters might be changed
> silently since they are copied to the new process's stack continuously.
> 
> Fixes: ca6eaaa210de ("riscv: __asm_copy_to-from_user: Optimize unaligned memory access and pipeline stall")
> Signed-off-by: Qiu Wenbo <qiuwenbo@kylinos.com.cn>
> ---
>  arch/riscv/lib/uaccess.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
> index bceb0629e440..7ab7cb96dcd9 100644
> --- a/arch/riscv/lib/uaccess.S
> +++ b/arch/riscv/lib/uaccess.S
> @@ -36,6 +36,9 @@ ENTRY(__asm_copy_from_user)
>  	 * Use byte copy only if too small.
>  	 */
>  	li	a3, 8*SZREG /* size must be larger than size in word_copy */
> +	neg	t1, a0
> +	andi	t1, t1, SZREG-1
> +	add	a3, a3, t1
>  	bltu	a2, a3, .Lbyte_copy_tail
>  
>  	/*
> 

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