linux-kernel - Re: [PATCH v6 3/5] riscv: uaccess: use input constraints for ptr of __put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6cefe9be-c103-4533-9f44-4666d3fba2f5@ghiti.fr>
Date: Tue, 22 Apr 2025 14:10:39 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Cyril Bur <cyrilbur@...storrent.com>, palmer@...belt.com,
 aou@...s.berkeley.edu, paul.walmsley@...ive.com, charlie@...osinc.com,
 jrtc27@...c27.com, ben.dooks@...ethink.co.uk
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 jszhang@...nel.org
Subject: Re: [PATCH v6 3/5] riscv: uaccess: use input constraints for ptr of
 __put_user()


On 10/04/2025 09:05, Cyril Bur wrote:
> From: Jisheng Zhang <jszhang@...nel.org>
>
> Putting ptr in the inputs as opposed to output may seem incorrect but
> this is done for a few reasons:
> - Not having it in the output permits the use of asm goto in a
>    subsequent patch. There are bugs in gcc [1] which would otherwise
>    prevent it.
> - Since the output memory is userspace there isn't any real benefit from
>    telling the compiler about the memory clobber.
> - x86, arm and powerpc all use this technique.
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113921 # 1
>
> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
> [Cyril Bur: Rewritten commit message]
> Signed-off-by: Cyril Bur <cyrilbur@...storrent.com>
> ---
>   arch/riscv/include/asm/uaccess.h | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index c9a461467bf4..da36057847f0 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -219,11 +219,11 @@ do {								\
>   	__typeof__(*(ptr)) __x = x;				\
>   	__asm__ __volatile__ (					\
>   		"1:\n"						\
> -		"	" insn " %z2, %1\n"			\
> +		"	" insn " %z1, %2\n"			\
>   		"2:\n"						\
>   		_ASM_EXTABLE_UACCESS_ERR(1b, 2b, %0)		\
> -		: "+r" (err), "=m" (*(ptr))			\
> -		: "rJ" (__x));					\
> +		: "+r" (err)					\
> +		: "rJ" (__x), "m"(*(ptr)));			\
>   } while (0)
>   
>   #ifdef CONFIG_64BIT
> @@ -236,16 +236,16 @@ do {								\
>   	u64 __x = (__typeof__((x)-(x)))(x);			\
>   	__asm__ __volatile__ (					\
>   		"1:\n"						\
> -		"	sw %z3, %1\n"				\
> +		"	sw %z1, %3\n"				\
>   		"2:\n"						\
> -		"	sw %z4, %2\n"				\
> +		"	sw %z2, %4\n"				\
>   		"3:\n"						\
>   		_ASM_EXTABLE_UACCESS_ERR(1b, 3b, %0)		\
>   		_ASM_EXTABLE_UACCESS_ERR(2b, 3b, %0)		\
> -		: "+r" (err),					\
> -			"=m" (__ptr[__LSW]),			\
> -			"=m" (__ptr[__MSW])			\
> -		: "rJ" (__x), "rJ" (__x >> 32));		\
> +		: "+r" (err)					\
> +		: "rJ" (__x), "rJ" (__x >> 32),			\
> +			"m" (__ptr[__LSW]),			\
> +			"m" (__ptr[__MSW]));			\
>   } while (0)
>   #endif /* CONFIG_64BIT */
>   


Reviewed-by: Alexandre Ghiti <alexghiti@...osinc.com>

Thanks,

Alex