> Date: Mon, 03 Mar 2025 06:50:47 +0700
> From: Robert Elz <k...@munnari.oz.au>
>
> Date: Sun, 2 Mar 2025 22:46:24 +0000
> From: "Taylor R Campbell" <riastr...@netbsd.org>
> Message-ID: <20250302224624.1a707f...@cvs.netbsd.org>
>
> | Log Message:
> | libc: New __libc_atfork.
> |
> | This uses caller-provided storage for the callback queues.
> |
> | Use it in arc4random(3) in order to avoid possible failure modes.
>
> This is the wrong solution, it exposes the _atfork data struct outside
> pthread_atfork.c which will make it essentially impossible to ever alter
> the internals of that interface.
It's not `wrong'; it's a legitimate way to eliminate unnecessary
failure modes, weighing costs of one approach vs costs of another.
But it _is_ wrong for pthread_atfork to fail in pthread__init or
pthread_tsd_init.
The original motivation of __libc_atfork is that pthread__init and
pthread_tsd_init cannot fail, but we currently have no way to
guarantee that because pthread_atfork can fail. They are called as
part of an ELF constructor, but other applications could call
pthread_atfork in their own ELF constructors, and there is no ordering
on ELF constructors. (I then went looking for other pthread_atfork
callers and noted the arc4random issues.)
We can preallocate some pthread_atfork records (and use a custom
allocator with some nicely simplifying properties of the caller to
avoid malloc), but that doesn't guarantee anything as long as
applications ever call pthread_atfork in an ELF constructor.
Now, the only reason pthread_atfork might fail is storage allocation
failure. With __libc_atfork, by having the caller preallocate it up
front, we can easily guarantee it does not fail. The cost of this
guarantee is compatibility constraints on __libc_atfork, but in this
case that's a small cost.
What I implemented and tested at first is the most trivial
proof-of-concept that
(a) doesn't require any changes to the calling side (in fork), and
(b) doesn't actually create any compatibility constraints because
nothing outside libc uses it yet.
I'm happy to replace it by another signature that only has one extra
argument instead of three, and still guarantees no failure, before we
use it in libpthread. We can define the interface so that the caller
has to allocate the larger of (say) eight aligned words per call or
space for three function pointers and two pointers, which is surely
enough to satisfy any reasonable implementation of fork. The worst
case I can imagine is that we want to switch pthread_atfork to an
array of handlers instead of a linked list; in that case we would
still have to support the linked list for __libc_atfork.
> arc4random() should simply cope with pthread_atfork() failing, which its
> interface spec says it is entitled to do, and which all other callers need
> to deal with.
>
> I do agree with PR lib/59117 that abort() is not a suitable "deal with"
> though, but there are other ways (slower perhaps) of generating pseudo-random
> bit strings (of suitable quality) that do not require the services of
> pthread_atfork(). We should simply do that.
arc4random can be implemented to handle pthread_atfork failure
gracefully, but this is a security-critical component that already has
a lot of paths to test. We can eliminate the need for some of those
paths, and for additional audit and testing, by simply providing a
__libc_atfork that does not fail.
