linux-kernel - Re: [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper

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: <20250605091112-7cd6b3bd-a466-486a-aebc-7bf0b2a8ac31@linutronix.de>
Date: Thu, 5 Jun 2025 09:15:36 +0200
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Xi Ruoyao <xry111@...111.site>
Cc: Alexandre Ghiti <alex@...ti.fr>, Nathan Chancellor <nathan@...nel.org>, 
	"Jason A. Donenfeld" <Jason@...c4.com>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Guo Ren <guoren@...nel.org>, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] RISC-V: vDSO: Correct inline assembly constraints in the
 getrandom syscall wrapper

Hi Xi,

On Wed, Jun 04, 2025 at 11:09:52PM +0800, Xi Ruoyao wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.

Good catch, I forgot to check the getrandom() implementations.
 
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  I'm not sure if using a simple
> "r" for the input is safe or not here, so just follow the documentation.

This looks good in general. However the usage of the "0" constraint now differs
from the "r" constraint used by all other vDSO implementations, even the RISC-V
gettimeofday() itself.
I'd like to hold off applying this commit and wait for the conclusion of the
discussion linked below and then implement the result of that everywhere.

> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> Cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Signed-off-by: Xi Ruoyao <xry111@...111.site>
> ---
>  arch/riscv/include/asm/vdso/getrandom.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> index 8dc92441702a..0d84a38e79da 100644
> --- a/arch/riscv/include/asm/vdso/getrandom.h
> +++ b/arch/riscv/include/asm/vdso/getrandom.h
> @@ -18,8 +18,8 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  	register unsigned int flags asm("a2") = _flags;
>  
>  	asm volatile ("ecall\n"
> -		      : "+r" (ret)
> -		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
> +		      : "=r" (ret)
> +		      : "r" (nr), "0" (buffer), "r" (len), "r" (flags)
>  		      : "memory");
>  
>  	return ret;
> 
> base-commit: dc5240f09bca7b5fc72ad8894d6b9321bce51139
> -- 
> 2.49.0