NetBSD Problem Report #59124

From www@netbsd.org  Mon Mar  3 21:02:23 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 919D21A923A
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  3 Mar 2025 21:02:23 +0000 (UTC)
Message-Id: <20250303210222.688CD1A923D@mollari.NetBSD.org>
Date: Mon,  3 Mar 2025 21:02:22 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: arc4random(3): first call in process races with concurrent fork
X-Send-Pr-Version: www-1.0

>Number:         59124
>Category:       lib
>Synopsis:       arc4random(3): first call in process races with concurrent fork
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    riastradh
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Mar 03 21:05:00 +0000 2025
>Closed-Date:    
>Last-Modified:  Thu Mar 13 10:22:30 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NetBSD Fork4randamnation
>Environment:
>Description:
Suppose thread A calls arc4random for the first time, and thread B calls fork at the same time.

arc4random will (a) take the global arc4random lock and (b) call pthread_atfork to make itself fork-safe.

But if the fork happens between (a) and (b), the child will see the lock held with no threads running to release it.
>How-To-Repeat:
#include <sys/wait.h>

#include <err.h>
#include <pthread.h>
#include <stdlib.h>
#include <unistd.h>

static void *
forkit(void *cookie)
{
	pthread_barrier_t *bar = cookie;
	pid_t pid;
	int status;

	(void)pthread_barrier_wait(bar);
	if ((pid = fork()) == -1)
		err(1, "fork");
	if (pid == 0) {
		(void)alarm(1);
		(void)arc4random();
		_exit(0);
	}
	if (waitpid(pid, &status, 0) == -1)
		err(1, "waitpid");
	if (WIFSIGNALED(status)) {
		errx(1, "child exited on signal %d (%s)", WTERMSIG(status),
		    strsignal(WTERMSIG(status)));
	}
	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
		errx(1, "child exited 0x%x", status);
	return NULL;
}

static void
test(void)
{
	pthread_barrier_t bar;
	pthread_t t;
	int error;

	error = pthread_barrier_init(&bar, NULL, 2);
	if (error)
		errc(1, error, "pthread_barrier_init");
	error = pthread_create(&t, NULL, &forkit, &bar);
	if (error)
		errc(1, error, "pthread_create");
	(void)pthread_barrier_wait(&bar);
	(void)arc4random();
	error = pthread_join(t, NULL);
	if (error)
		errc(1, error, "pthread_join");
}

int
main(void)
{

	for (unsigned long long n = 1;; n++) {
		pid_t pid;
		int status;

		if ((pid = fork()) == -1)
			err(1, "fork");
		if (pid == 0) {
			test();
			_exit(0);
		}
		if (waitpid(pid, &status, 0) == -1)
			err(1, "waitpid");
		if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
			errx(1, "test exited 0x%x after %zu trial%s", status,
			    n, n == 1 ? "" : "s");
		}
	}
	return 0;
}
>Fix:
Yes, please!

This is an instance of a more general problem.  One way we could address this is by creating a variant of pthread_atfork with caller-supplied storage, and using it in an ELF constructor routine which runs before any threads can be started.  Applications, of course, can avoid the problem if they make a single call to arc4random before fork.  But this has never been part of the contract of arc4random which is generally to be usable in any contexts without having to think about anything.

>Release-Note:

>Audit-Trail:
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/59124: arc4random(3): first call in process races with concurrent fork
Date: Tue, 04 Mar 2025 10:29:36 +0700

     Date:        Mon,  3 Mar 2025 21:05:00 +0000 (UTC)
     From:        campbell+netbsd@mumble.net
     Message-ID:  <20250303210500.63C301A923F@mollari.NetBSD.org>

   | arc4random will (a) take the global arc4random lock and (b) call pthre=
 ad_atfork to make itself fork-safe.
   |
   | But if the fork happens between (a) and (b), the child will see the lo=
 ck held with no threads running to release it.


   | One way we could address this is by creating a variant of pthread_atfo=
 rk
   | with caller-supplied storage, and using it in an ELF constructor routi=
 ne

 Or, arc4random() could just call pthread_atfork() before it takes its own
 private lock.   That has to be simpler.   Just reverse (a) and (b).

 All that's needed is for the callback functions it supplies to able to tel=
 l
 if the lock is held or not, before attempting to release it - which is nee=
 ded
 anyway, as a more likely case is some thread forking, sometime after a cal=
 l
 to arc4random, when no call to that is active, and no lock is held.

 kre

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59124 CVS commit: src/lib/libc
Date: Thu, 6 Mar 2025 00:53:27 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu Mar  6 00:53:27 UTC 2025

 Modified Files:
 	src/lib/libc/gen: arc4random.c
 	src/lib/libc/include: arc4random.h

 Log Message:
 arc4random(3): Switch to use thr_once (libc pthread_once symbol).

 This way, we reduce the problem of arc4random initialization
 fork-safety to the problem of pthread_once fork-safety -- and
 anything else to do with one-time lazy initialization.

 PR lib/59124: arc4random(3): first call in process races with
 concurrent fork


 To generate a diff of this commit:
 cvs rdiff -u -r1.43 -r1.44 src/lib/libc/gen/arc4random.c
 cvs rdiff -u -r1.2 -r1.3 src/lib/libc/include/arc4random.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: lib-bug-people->riastradh
Responsible-Changed-By: riastradh@NetBSD.org
Responsible-Changed-When: Thu, 13 Mar 2025 10:22:30 +0000
Responsible-Changed-Why:
reduced to a separately solvable problem in HEAD
needs pullup-10, pullup-9


State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Thu, 13 Mar 2025 10:22:30 +0000
State-Changed-Why:
mine


>Unformatted:

Home	PR Database Search