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: <3080562c-7cfd-4f66-9f62-9a99552d283f@t-8ch.de>
Date: Mon, 17 Feb 2025 22:41:04 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
Cc: Willy Tarreau <w@....eu>, 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

Hi Maciej,

thanks for your feedback!

On 2025-02-16 15:41:55+0000, Maciej W. Rozycki wrote:
> On Wed, 12 Feb 2025, Thomas Weißschuh wrote:
> > diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
> > index 753a8ed2cf695f0b5eac4b5e4d317fdb383ebf93..638520a3427a985fdbd5f5a49b55853bbadeee75 100644
> > --- a/tools/include/nolibc/arch-mips.h
> > +++ b/tools/include/nolibc/arch-mips.h
> > @@ -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
> > +		"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
> 
>  Why is this code breaking stack alignment just to have to fix it up two 
> instructions down the line?  Or is it that the incoming $sp is not aligned 
> in the first place (in which case we're having a deeper problem).

nolibc itself does not assume that $sp is aligned.
Maybe Willy can explain the historical background.

The System V ABI MIPS supplement [0] says the following:

The registers listed below have the specified contents at process entry:
	...

	$sp The stack pointer holds the address of the bottom of the stack, which
	must be doubleword (8 byte) aligned.
	...

However "process entry" is main(), while this code is running before main.

The kernel always aligns the stack to a multiple of 16 bytes.
See the usage of STACK_ROUND() in fs/binfmt_elf.c.

So I guess we could remove the manual alignment.
(At least for alignments of 16 bytes and less)

> > +
> > +		/* 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
> > +		"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"
> 
>  This could be optimised using a temporary (e.g. $at, but I guess any will 
> do as I gather we don't have any ABI abnormalities here).

clang rejects manual usage of $at without ".set noat".
So $t0 is simpler.

> > +#endif
> > +
> >  		"jalr $t9\n"             /* transfer to c runtime
> > */
> >  		" nop\n"                 /* delayed slot
> 
>  On an unrelated matter JALR above ought to be JAL (or otherwise there's 
> no point in using the .cprestore pseudo-op).  And I fail to see why this 
> code has to be "noreorder" (except for the .cpload piece, of course), it's 
> just asking for troubles.

Thanks for the hints.

Without "noreorder", is the manually addition of the delayed slot "nop"
still necessary?
These points also apply to the existing O32 implementation, right?
If so I'll make a proper series out of it.

[0] https://refspecs.linuxfoundation.org/elf/mipsabi.pdf


Thomas