linux-kernel - Re: [PATCH] tools/nolibc: add support for N64 and N32 ABIs

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: <20250216094938.GB2192@1wt.eu>
Date: Sun, 16 Feb 2025 10:49:38 +0100
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Shuah Khan <shuah@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-mips@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH] tools/nolibc: add support for N64 and N32 ABIs

On Wed, Feb 12, 2025 at 07:49:53PM +0100, Thomas Weißschuh wrote:
> +#if defined(_ABIO32)
> +
>  #define _NOLIBC_SYSCALL_CLOBBERLIST \
>  	"memory", "cc", "at", "v1", "hi", "lo", \
>  	"t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9"
> +#define _NOLIBC_SYSCALL_STACK_RESERVE "addiu $sp, $sp, -32\n"
> +#define _NOLIBC_SYSCALL_STACK_UNRESERVE "addiu $sp, $sp, 32\n"
> +
> +#elif defined(_ABIN32) || defined(_ABI64)
> +
> +/* binutils, GCC and clang disagree about register aliases, use numbers instead. */

Is this often encountered despite this ? I guess it can cause portability
issues :-/

> +#if defined(_ABIO32)
> +
>  #define my_syscall5(num, arg1, arg2, arg3, arg4, arg5)                        \
>  ({                                                                            \
>  	register long _num __asm__ ("v0") = (num);                            \
(...)
> @@ -178,6 +201,50 @@
>  	_arg4 ? -_num : _num;                                                 \
>  })
>  
> +#else
> +

Here you should indicate which ABI is covered by this #else, because one
has to go up to previous definitions to figure it's _ABIN32 and _ABI64.

> +#define my_syscall5(num, arg1, arg2, arg3, arg4, arg5)                        \
> +({                                                                            \
> +	register long _num __asm__ ("v0") = (num);                            \
> +	register long _arg1 __asm__ ("$4") = (long)(arg1);                    \
> +	register long _arg2 __asm__ ("$5") = (long)(arg2);                    \
> +	register long _arg3 __asm__ ("$6") = (long)(arg3);                    \
> +	register long _arg4 __asm__ ("$7") = (long)(arg4);                    \
> +	register long _arg5 __asm__ ("$8") = (long)(arg5);                    \
(...)
> @@ -190,13 +257,33 @@ void __attribute__((weak, noreturn)) __nolibc_entrypoint __no_stack_protector __
>  		"1:\n"
>  		".cpload $ra\n"
>  		"move  $a0, $sp\n"       /* save stack pointer to $a0, as arg1 of _start_c */
> +
> +#if defined(_ABIO32)
>  		"addiu $sp, $sp, -4\n"   /* space for .cprestore to store $gp              */
>  		".cprestore 0\n"
>  		"li    $t0, -8\n"
>  		"and   $sp, $sp, $t0\n"  /* $sp must be 8-byte aligned                     */
>  		"addiu $sp, $sp, -16\n"  /* the callee expects to save a0..a3 there        */
> -		"lui $t9, %hi(_start_c)\n" /* ABI requires current function address in $t9 */
> +#else

So same here. I think you should do it for all #else since you're generally
grouping 2 ABIs vs one between a #if and a #else and it's not trivial to
figure what a #else covers, like below.

> +		"daddiu $sp, $sp, -8\n"  /* space for .cprestore to store $gp              */
> +		".cpsetup $ra, 0, 1b\n"
> +		"li    $t0, -16\n"
> +		"and   $sp, $sp, $t0\n"  /* $sp must be 16-byte aligned                    */
> +#endif
> +
> +		/* ABI requires current function address in $t9 */
> +#if defined(_ABIO32) || defined(_ABIN32)
> +		"lui $t9, %hi(_start_c)\n"
>  		"ori $t9, %lo(_start_c)\n"
> +#else

This one indeed covers only _ABI64

> +		"lui  $t9, %highest(_start_c)\n"
> +		"ori  $t9, %higher(_start_c)\n"
> +		"dsll $t9, 0x10\n"
> +		"ori  $t9, %hi(_start_c)\n"
> +		"dsll $t9, 0x10\n"
> +		"ori  $t9, %lo(_start_c)\n"
> +#endif

With the tiny details above, this looks fine. It's great that syscall
numbers didn't change so that you can cover an extra arch with only a
few ifdefs. I have not tested but I guess you did :-) So that's OK for
me:

Acked-by: Willy Tarreau <w@....eu>

Thanks!
Willy