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: <CAAhV-H68vDzU7XPefDYG+MRMs560q8o2GhiqYRm1hsA+3DND3A@mail.gmail.com>
Date: Thu, 5 Jun 2025 19:18:42 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Xi Ruoyao <xry111@...111.site>, 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 Thu, Jun 5, 2025 at 3:37 PM Thomas Weißschuh
<thomas.weissschuh@...utronix.de> wrote:
>
> 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.
OK, if there are no objections, I will take this patch.

Huacai

>
>
> Thomas