linux-kernel - Re: [PATCH 3/3] tools/nolibc: x86_64: wrap asm functions in functions

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: <65cf1965-841b-4fb1-9222-913c6bd9ccd4@t-8ch.de>
Date: Mon, 12 Aug 2024 19:01:26 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Willy Tarreau <w@....eu>
Cc: Ammar Faizi <ammarfaizi2@...weeb.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] tools/nolibc: x86_64: wrap asm functions in functions

On 2024-08-10 19:00:30+0000, Willy Tarreau wrote:
> On Sat, Aug 10, 2024 at 06:45:19PM +0200, Thomas Weißschuh wrote:
> > Iff we really want to support it, we could do use naked where available
> > and fall back to toplevel asm otherwise.
> > This should work on newer compilers and older ones without -flto.
> > It looks horrible though.
> > 
> >   #define NOLIBC_ARCH_HAS_MEMSET
> >   void *memset(void *dst, int c, size_t len);
> >   
> >   #if __nolibc_has_attribute(naked)
> > 
> >   __attribute__((weak,naked))
> >   void *memset(void *dst __attribute__((unused)), int c __attribute__((unused)), size_t len __attribute__((unused))) {
> > 
> >   #else
> > 
> >   __asm__ (
> >   ".section .text.nolibc_memset\n"
> >   ".weak memset\n"
> >   "memset:\n"
> >   );
> > 
> >   #endif
> > 
> >   __asm__ (
> >   	"xchgl %eax, %esi\n\t"
> >   	"movq  %rdx, %rcx\n\t"
> >   	"pushq %rdi\n\t"
> >   	"rep stosb\n\t"
> >   	"popq  %rax\n\t"
> >   	"retq\n"
> >   );
> > 
> >   #if __nolibc_has_attribute(naked)
> >   }
> >   #endif
> > 
> > (Or some impenetrable macro wrapper abstraction thereof)
> 
> One dangerous part above is that the compiler can reorder toplevel asm
> statements, so having a label in one and the code in another may result
> in random bugs.

Indeed, let's scratch that then.

> > The memcpy / memmove combination could be split up into one real
> > function and one C inline wrapper and then the same pattern would apply.
> > 
> > But to be honest I'd be fine with not supporting -flto on GCC.
> 
> That could also be a reasonable solution. The primary goal of nolibc
> is to make it easy for developers to develop tests, and for those who
> want to create pre-boot code to do so. By nature this code is prone to
> bugs. If it becomes totally unreadable for very unlikely cases, it will
> cause issues that are hard to debug by the users themselves. It's sure
> that supporting a variety of compilers and setups is great, but we should
> keep in mind the maintainability goal when thinking about this. I think
> that LTO will mostly be used for testing, and in this case I think it's
> totally reasonable to restrict the choice of compatible compilers.

IMO LTO is useful for nolibc to reduce the size of the generated binaries.
For example the custom function sections that then can be combined with
--gc-sections to get rid of unused functions is all implicit in LTO.
And on top of that it can perform other size optimizations.

But the complications for GCC are not worth it.
And there seem general with LTO on GCC anyways as evidenced by the
constructor and duplicate-asm bugs.

After this discussion I think we should also change the __nolibc_naked
attribute back to __nolibc_entrypoint as it was before.
It is indeed not a replacement for a proper "naked".
What do you think about me rewriting the existing commits on nolibc-next
to fix this up?


Thomas