Re: lib/59148: arc4random calls malloc so it can't be used in an ELF constructor

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The attached patches address this in two alternative ways:

1. [pr59148-arc4randomsysctlqueryconstructor.patch] Resolve the sysctl
   kern.entropy.epoch mib in an ELF constructor, where the stack usage
   is fairly shallow so it's not too onerous to allocate >12KB(!) on
   the stack for listing the kern.* children.  I'm not really happy
   about this because it incurs the cost for all applications, even
   those that don't call arc4random, because it does the work in a
   constructor.

2. [pr59148-arc4randomsysctlstaticnum.patch] Statically assign sysctl
   mib numbers to kern.entropy.epoch, so userland can just query
   {CTL_KERN, KERN_ENTROPY, KERN_ENTROPY_EPOCH}.  It's annoying to
   have to statically assign new numbers but not really that big a
   deal and I think I'm willing to argue that kern.entropy.epoch is a
   long-term useful thing that is worth committing to.

Ideally, we would eliminate this whole sysctl lookup path in favour of
reading a single word out of a vDSO mapped in userland, but that
requires the whole vDSO machinery which we don't have yet.  I'm
leaning toward (2).

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1741308823 0
#      Fri Mar 07 00:53:43 2025 +0000
# Branch trunk
# Node ID 1c29ada773b0a0e9ad17221151d839be0e711eb9
# Parent  e3b67aeee565934995180b3b1a684fa2ea3c47b4
# EXP-Topic riastradh-pr59117-arc4randomfail
WIP: arc4random(3): Resolve kern.entropy.epoch sysctl without malloc.

PR lib/59148: arc4random calls malloc so it can't be used in an ELF
constructor

diff -r e3b67aeee565 -r 1c29ada773b0 lib/libc/gen/arc4random.c
--- a/lib/libc/gen/arc4random.c	Fri Mar 07 01:22:46 2025 +0000
+++ b/lib/libc/gen/arc4random.c	Fri Mar 07 00:53:43 2025 +0000
@@ -64,7 +64,6 @@
 
 #include <assert.h>
 #include <sha2.h>
-#include <stdatomic.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -480,6 +479,79 @@ crypto_onetimestream_selftest(void)
 	return 0;
 }
 
+static int kern_entropy_epoch_mib[3];
+
+__attribute__((constructor))
+__section(".text.startup")
+static void
+entropy_epoch_init(void)
+{
+	struct sysctlnode query, nodes[128]; /* XXX 12k of stack space */
+	size_t i, len;
+
+	/*
+	 * Before constructors have run, this may be called multiple
+	 * times in case a caller uses arc4random from a constructor --
+	 * but only from a single thread, so no synchronization is
+	 * needed.
+	 *
+	 * After constructors have run, kern_entropy_epoch_mib is
+	 * stable, because this is one of the constructors.
+	 */
+	if (__predict_true(kern_entropy_epoch_mib[0] != 0))
+		return;
+
+	/*
+	 * Start with CTL_KERN which is statically assigned.
+	 */
+	kern_entropy_epoch_mib[0] = CTL_KERN;
+
+	/*
+	 * Resolve kern.entropy.
+	 */
+	kern_entropy_epoch_mib[1] = CTL_QUERY;
+	memset(&query, 0, sizeof(query));
+	query.sysctl_flags = SYSCTL_VERSION;
+	len = sizeof(nodes);
+	if (sysctl(kern_entropy_epoch_mib, 2, nodes, &len,
+		&query, sizeof(query)) == -1)
+		goto fail;
+	for (i = 0; i < len/sizeof(nodes[0]); i++) {
+		if (strcmp(nodes[i].sysctl_name, "entropy") == 0) {
+			kern_entropy_epoch_mib[1] = nodes[i].sysctl_num;
+			break;
+		}
+	}
+	if (i == len/sizeof(nodes[0]))
+		goto fail;
+
+	/*
+	 * Resolve kern.entropy.epoch.
+	 */
+	kern_entropy_epoch_mib[2] = CTL_QUERY;
+	memset(&query, 0, sizeof(query));
+	query.sysctl_flags = SYSCTL_VERSION;
+	len = sizeof(nodes);
+	if (sysctl(kern_entropy_epoch_mib, 3, nodes, &len,
+		&query, sizeof(query)) == -1)
+		goto fail;
+	for (i = 0; i < len/sizeof(nodes[0]); i++) {
+		if (strcmp(nodes[i].sysctl_name, "epoch") == 0) {
+			kern_entropy_epoch_mib[2] = nodes[i].sysctl_num;
+			break;
+		}
+	}
+	if (i == len/sizeof(nodes[0]))
+		goto fail;
+
+	/*
+	 * Success!
+	 */
+	return;
+
+fail:	kern_entropy_epoch_mib[0] = CTL_EOL;
+}
+
 /*
  * entropy_epoch()
  *
@@ -499,36 +571,15 @@ crypto_onetimestream_selftest(void)
 static unsigned
 entropy_epoch(void)
 {
-	static atomic_int mib0[3];
-	static atomic_bool initialized = false;
-	int mib[3];
 	unsigned epoch = (unsigned)-1;
 	size_t epochlen = sizeof(epoch);
 
-	/*
-	 * Resolve kern.entropy.epoch if we haven't already.  Cache it
-	 * for the next caller.  Initialization is idempotent, so it's
-	 * OK if two threads do it at once.
-	 */
-	if (atomic_load_explicit(&initialized, memory_order_acquire)) {
-		mib[0] = atomic_load_explicit(&mib0[0], memory_order_relaxed);
-		mib[1] = atomic_load_explicit(&mib0[1], memory_order_relaxed);
-		mib[2] = atomic_load_explicit(&mib0[2], memory_order_relaxed);
-	} else {
-		size_t nmib = __arraycount(mib);
-
-		if (sysctlnametomib("kern.entropy.epoch", mib, &nmib) == -1)
-			return (unsigned)-1;
-		if (nmib != __arraycount(mib))
-			return (unsigned)-1;
-		atomic_store_explicit(&mib0[0], mib[0], memory_order_relaxed);
-		atomic_store_explicit(&mib0[1], mib[1], memory_order_relaxed);
-		atomic_store_explicit(&mib0[2], mib[2], memory_order_relaxed);
-		atomic_store_explicit(&initialized, true,
-		    memory_order_release);
-	}
-
-	if (sysctl(mib, __arraycount(mib), &epoch, &epochlen, NULL, 0) == -1)
+	entropy_epoch_init();
+	if (kern_entropy_epoch_mib[0] == CTL_EOL)
+		return (unsigned)-1;
+	if (sysctl(kern_entropy_epoch_mib,
+		__arraycount(kern_entropy_epoch_mib),
+		&epoch, &epochlen, NULL, 0) == -1)
 		return (unsigned)-1;
 	if (epochlen != sizeof(epoch))
 		return (unsigned)-1;

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1741314350 0
#      Fri Mar 07 02:25:50 2025 +0000
# Branch trunk
# Node ID c8aac91a801fc586d432bf9164a68934a0902580
# Parent  e3b67aeee565934995180b3b1a684fa2ea3c47b4
# EXP-Topic riastradh-pr59117-arc4randomfail
WIP: Assign static MIB numbers for kern.entropy.epoch.

This sidesteps any need for dynamically sized memory in userland to
resolve sysctl names to read it out, or for a new syscall interface
to sysctl resolution by name.

It would really be better to expose this through a page shared with
userland, so querying it doesn't cost a syscall, but this will serve
for now.

PR lib/59148: arc4random calls malloc so it can't be used in an ELF
constructor

diff -r e3b67aeee565 -r c8aac91a801f lib/libc/gen/arc4random.c
--- a/lib/libc/gen/arc4random.c	Fri Mar 07 01:22:46 2025 +0000
+++ b/lib/libc/gen/arc4random.c	Fri Mar 07 02:25:50 2025 +0000
@@ -64,7 +64,6 @@
 
 #include <assert.h>
 #include <sha2.h>
-#include <stdatomic.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -499,35 +498,10 @@ crypto_onetimestream_selftest(void)
 static unsigned
 entropy_epoch(void)
 {
-	static atomic_int mib0[3];
-	static atomic_bool initialized = false;
-	int mib[3];
+	const int mib[] = { CTL_KERN, KERN_ENTROPY, KERN_ENTROPY_EPOCH };
 	unsigned epoch = (unsigned)-1;
 	size_t epochlen = sizeof(epoch);
 
-	/*
-	 * Resolve kern.entropy.epoch if we haven't already.  Cache it
-	 * for the next caller.  Initialization is idempotent, so it's
-	 * OK if two threads do it at once.
-	 */
-	if (atomic_load_explicit(&initialized, memory_order_acquire)) {
-		mib[0] = atomic_load_explicit(&mib0[0], memory_order_relaxed);
-		mib[1] = atomic_load_explicit(&mib0[1], memory_order_relaxed);
-		mib[2] = atomic_load_explicit(&mib0[2], memory_order_relaxed);
-	} else {
-		size_t nmib = __arraycount(mib);
-
-		if (sysctlnametomib("kern.entropy.epoch", mib, &nmib) == -1)
-			return (unsigned)-1;
-		if (nmib != __arraycount(mib))
-			return (unsigned)-1;
-		atomic_store_explicit(&mib0[0], mib[0], memory_order_relaxed);
-		atomic_store_explicit(&mib0[1], mib[1], memory_order_relaxed);
-		atomic_store_explicit(&mib0[2], mib[2], memory_order_relaxed);
-		atomic_store_explicit(&initialized, true,
-		    memory_order_release);
-	}
-
 	if (sysctl(mib, __arraycount(mib), &epoch, &epochlen, NULL, 0) == -1)
 		return (unsigned)-1;
 	if (epochlen != sizeof(epoch))
diff -r e3b67aeee565 -r c8aac91a801f sys/kern/kern_entropy.c
--- a/sys/kern/kern_entropy.c	Fri Mar 07 01:22:46 2025 +0000
+++ b/sys/kern/kern_entropy.c	Fri Mar 07 02:25:50 2025 +0000
@@ -358,7 +358,7 @@ entropy_init(void)
 	    CTLFLAG_PERMANENT, CTLTYPE_NODE, "entropy",
 	    SYSCTL_DESCR("Entropy (random number sources) options"),
 	    NULL, 0, NULL, 0,
-	    CTL_KERN, CTL_CREATE, CTL_EOL);
+	    CTL_KERN, KERN_ENTROPY, CTL_EOL);
 
 	/* Create the sysctl knobs.  */
 	/* XXX These shouldn't be writable at securelevel>0.  */
@@ -402,7 +402,7 @@ entropy_init(void)
 	sysctl_createv(&entropy_sysctllog, 0, &entropy_sysctlroot, NULL,
 	    CTLFLAG_PERMANENT|CTLFLAG_READONLY, CTLTYPE_INT,
 	    "epoch", SYSCTL_DESCR("Entropy epoch"),
-	    NULL, 0, &E->epoch, 0, CTL_CREATE, CTL_EOL);
+	    NULL, 0, &E->epoch, 0, KERN_ENTROPY_EPOCH, CTL_EOL);
 
 	/* Initialize the global state for multithreaded operation.  */
 	mutex_init(&E->lock, MUTEX_DEFAULT, IPL_SOFTSERIAL);
diff -r e3b67aeee565 -r c8aac91a801f sys/sys/sysctl.h
--- a/sys/sys/sysctl.h	Fri Mar 07 01:22:46 2025 +0000
+++ b/sys/sys/sysctl.h	Fri Mar 07 02:25:50 2025 +0000
@@ -275,6 +275,7 @@ struct ctlname {
 #define	KERN_BOOTTIME		83	/* struct: time kernel was booted */
 #define	KERN_EVCNT		84	/* struct: evcnts */
 #define	KERN_SOFIXEDBUF		85	/* bool: fixed socket buffer sizes */
+#define	KERN_ENTROPY		86	/* node: entropy(9) subsystem */
 
 /*
  *  KERN_CLOCKRATE structure
@@ -780,6 +781,12 @@ typedef int	(*hashstat_func_t)(struct ha
 void		hashstat_register(const char *, hashstat_func_t);
 
 /*
+ * kern.entropy.* variables
+ */
+
+#define	KERN_ENTROPY_EPOCH		1
+
+/*
  * CTL_VM identifiers in <uvm/uvm_param.h>
  */