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: <edca541e-a2a7-4c26-bea7-15fd0b25597b@xen0n.name>
Date: Thu, 5 Jun 2025 00:39:58 +0800
From: WANG Xuerui <kernel@...0n.name>
To: Huacai Chen <chenhuacai@...nel.org>,
 Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: 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>, Xi Ruoyao <xry111@...111.site>,
 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 6/4/25 22:05, 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/

Oops, I've already completely forgotten! That said...

I didn't notice back then, that `ret` and the first parameter actually 
shared the same manually allocated register, so I replied as if the two 
shared one variable. If it were me to write the original code, I would 
re-used `ret` for arg0 (with a comment explaining the a0 situation) so 
that "+r" could be properly used there without UB.

As for the current situation -- both this patch's approach or my 
alternative above are OK to me. Feel free to take either; and have my 
R-b tag if you send a v2.

Reviewed-by: WANG Xuerui <git@...0n.name>

Thanks!

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/