linux-kernel - Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall wrappers

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: <20250605092735-bd76e803-e896-4d4c-a1f1-c30f8d321a9a@linutronix.de>
Date: Thu, 5 Jun 2025 09:37:45 +0200
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Xi Ruoyao <xry111@...111.site>
Cc: Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>, 
	Theodore Ts'o <tytso@....edu>, "Jason A. Donenfeld" <Jason@...c4.com>, 
	Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <nick.desaulniers+lkml@...il.com>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Jiaxun Yang <jiaxun.yang@...goat.com>, loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	llvm@...ts.linux.dev, stable@...r.kernel.org
Subject: Re: [PATCH] LoongArch: vDSO: correctly use asm parameters in syscall
 wrappers

On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
> On Wed, 2025-06-04 at 22:05 +0800, Huacai Chen wrote:
> > On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh
> > <thomas.weissschuh@...utronix.de> wrote:
> > > 
> > > The syscall wrappers use the "a0" register for two different register
> > > variables, both the first argument and the return value. The "ret"
> > > variable is used as both input and output while the argument register is
> > > only used as input. Clang treats the conflicting input parameters as
> > > undefined behaviour and optimizes away the argument assignment.
> > > 
> > > The code seems to work by chance for the most part today but that may
> > > change in the future. Specifically clock_gettime_fallback() fails with
> > > clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
> > > 
> > > Switch the "ret" register variable to a pure output, similar to the other
> > > architectures' vDSO code. This works in both clang and GCC.
> > Hmmm, at first the constraint is "=r", during the progress of
> > upstream, Xuerui suggested me to use "+r" instead [1].
> > [1]  https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0n.name/
> 
> Based on the example at
> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
> 
>    To force an operand into a register, create a local variable and specify
>    the register name after the variable’s declaration. Then use the local
>    variable for the asm operand and specify any constraint letter that
>    matches the register:
>    
>    register int *p1 asm ("r0") = …;
>    register int *p2 asm ("r1") = …;
>    register int *result asm ("r0");
>    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
>    
> I think this should actually be written
> 
>  	asm volatile(
>  	"      syscall 0\n"
> 	: "=r" (ret)
>  	: "r" (nr), "0" (buffer), "r" (len), "r" (flags)
>  	: "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
> "$t8",
>  	  "memory");
> 
> i.e. "=" should be used for the output operand 0, and "0" should be used
> for the input operand 2 (buffer) to emphasis the same register as
> operand 0 is used.

I would have expected that matching constraints ("0") would only really make
sense if the compiler selects the specific register to use. When the register is
already selected manually it seems redundant.
But my inline ASM knowledge is limited and this is a real example from the GCC
docs, so it is probably more correct.
On the other hand all the other vDSO implementations use "r" over "0" for the
input operand 2 and I'd like to keep them consistent.


Thomas