Page MenuHomeFreeBSD

Make libsys self-contained
ClosedPublic

Authored by kib on Feb 20 2024, 3:21 PM.
Tags
None
Referenced Files
F89003517: D43985.diff
Mon, Jul 22, 6:31 AM
Unknown Object (File)
Tue, Jul 9, 6:42 PM
Unknown Object (File)
Jun 22 2024, 3:25 AM
Unknown Object (File)
Jun 22 2024, 2:11 AM
Unknown Object (File)
Jun 21 2024, 4:41 AM
Unknown Object (File)
May 20 2024, 1:51 PM
Unknown Object (File)
May 20 2024, 3:27 AM
Unknown Object (File)
May 20 2024, 3:27 AM
Subscribers

Details

Summary
rtld: remove pointless "extern"
libsys: internalize memcpy, memset, and strlcpy
libsys: remove usage of pthread_once and _once_stub

that existed in auxv.c, use simple bool gate instead. This leaves a
small window if two threads try to call _elf_aux_info(3) simultaneously.
The situation is safe because auxv parsing is really idempotent. The
parsed data is same, and we store atomic types (int/long/ptr) so
double-init does not matter.
libsys: move errno to libsys
libsys: disable ssp

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Feb 20 2024, 3:21 PM

So we're guaranteed to run the first time through single threaded?

I can't imagine how this would ever be multi-threaded and need the mutex_once treatment, so I'm wondering why it was there at all...

This revision is now accepted and ready to land.Feb 20 2024, 3:36 PM
In D43985#1003418, @imp wrote:

So we're guaranteed to run the first time through single threaded?

I can't imagine how this would ever be multi-threaded and need the mutex_once treatment, so I'm wondering why it was there at all...

I assume you mean the auxv parsing. No, it is not guaranteed to be run from single-threaded environment, but it should be safe since it is really idempotent. The parsed data is same, and we store atomic types (int/long/ptr) so double-init does not matter.

Thank you for following up on this. I'd had the same insight about init_aux() so glad to see it confirmed.

I'm not completely convinced we need to add memcpy.c and memset.c. In practice the runtime must provide them if there's any C at all because the compiler may generate calls, but it's fairly harmless to include the C versions.

lib/libsys/auxv.c
90

It might be worth a comment that this needs to remain idempotent (perhaps more specifically not subject to store tearing).

kib marked an inline comment as done.Feb 20 2024, 4:44 PM

I'm not completely convinced we need to add memcpy.c and memset.c. In practice the runtime must provide them if there's any C at all because the compiler may generate calls, but it's fairly harmless to include the C versions.

Without them, I get

/nm -D libsys.so.7 | awk '$1 == "U" {print $0}' 
                 U memcpy
                 U memset
                 U strlcpy

This comes from init_aux_vector_once:

0000000000000020 t init_aux_vector_once
                 U memcpy
                 U memset

Also it might make sense to compile libsys in freestanding environment. I wanted to enable it for libc, but for libsys it should be much less controversial.

Add comment to init_aux()

This revision now requires review to proceed.Feb 20 2024, 4:46 PM
This revision is now accepted and ready to land.Feb 20 2024, 5:00 PM

Change errno patch to fix static linking

This revision now requires review to proceed.Feb 20 2024, 7:20 PM
lib/libsys/auxv.c
56

For CheriABI, this is function becomes __elf_aux_vector = __auxargs; where __auxargs; is supplied by csu code because we pass the aux vector pointer in a register from the kernel rather than walking off the end of environ. I'll probably keep that for now, but I wonder if that's the right interface in the long term? I'd certainly rather this code not know about environ or the stack layout at all.

lib/libsys/auxv.c
56

Would we design the ABI from nil, then probably a register pointing to some extensible struct psargs (including argc, argv, environ, auxv) is the way to go.

But for the existing system, we have to keep current layout to allow older binaries to run.

Note that the code you pointed out is only used for static binaries, for dynamic, __elf_aux_vector is initialized by rtld (which caused all the troubles).

This revision was not accepted when it landed; it landed in state Needs Review.Feb 21 2024, 12:29 AM
This revision was automatically updated to reflect the committed changes.