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-H4Ba7DMV6AvGnvNBJ8FL_YcHjeeHYZWw2NG6JHL=X4PkQ@mail.gmail.com>
Date: Wed, 4 Jun 2025 22:05:37 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: 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>, 
	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 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/

Huacai

>
> Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd291a@linutronix.de/
> Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/
> Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support")
> Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation")
> Cc: stable@...r.kernel.org
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
> ---
>  arch/loongarch/include/asm/vdso/getrandom.h    | 2 +-
>  arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h
> index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644
> --- a/arch/loongarch/include/asm/vdso/getrandom.h
> +++ b/arch/loongarch/include/asm/vdso/getrandom.h
> @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>
>         asm volatile(
>         "      syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8",
>           "memory");
> diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h
> index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644
> --- a/arch/loongarch/include/asm/vdso/gettimeofday.h
> +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h
> @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (tv), "r" (tz)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
> @@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (clkid), "r" (ts)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
> @@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
>
>         asm volatile(
>         "       syscall 0\n"
> -       : "+r" (ret)
> +       : "=r" (ret)
>         : "r" (nr), "r" (clkid), "r" (ts)
>         : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7",
>           "$t8", "memory");
>
> ---
> base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253
> change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
>
> Best regards,
> --
> Thomas Weißschuh <thomas.weissschuh@...utronix.de>
>