Module Name:    src
Committed By:   riastradh
Date:           Tue Oct 15 17:34:06 UTC 2024

Modified Files:
        src/sys/crypto/cprng_fast: cprng_fast.c

Log Message:
Revert cprng_fast(9) to seed and reseed asynchronously in softint.

This reverts sys/crypto/cprng_fast/cprng_fast.c revisions 1.17-1.19.

I thought we had eliminated all paths into cprng_fast(9) from hard
interrupt context, which would allow us to call into cprng_strong(9)
and entropy(9) to synchronously reseed whenever needed -- this would
improve security over netbsd-9 for the first query to cprng_intr(9)
on each CPU.

Unfortunately, I missed the calls under spin locks (which are
effectively also hard interrupt context, in that they hold up
interrupts on this CPU or interrupt handlers trying to take the lock
on other CPUs).  And one such spin lock is struct ifnet::ifq_lock at
IPL_NET, which is held by if_transmit when it calls IFQ_ENQUEUE which
calls into altq(4) which sometimes does, e.g., red_addq which calls
cprng_fast32.

Until we migrate ifq_lock to IPL_SOFTNET (which is potentially
feasible, because most of the network stack runs in softint now, but
it requires a lot of auditing and maybe changes to lots of drivers),
we'll have to make sure cprng_fast(9) doesn't try to take an adaptive
lock.

And the simplest way to ensure that is to just revert back to the
netbsd-9 semantics of asynchronously reseeding in softint, at the
cost of a potential security weakness.  I don't expect this
regression to be permanent -- we just can't restore the change as is
until we deal with ifq_lock.

1.19 cprng_fast(9): Drop and retake percpu reference across cprng_strong.
1.18 cprng_fast(9): Assert not in pserialize read section.
1.17 cprng(9): cprng_fast is no longer used from interrupt context.

PR kern/58575: altq(4) takes adaptive lock while holding spin lock


To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/sys/crypto/cprng_fast/cprng_fast.c

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

Modified files:

Index: src/sys/crypto/cprng_fast/cprng_fast.c
diff -u src/sys/crypto/cprng_fast/cprng_fast.c:1.19 src/sys/crypto/cprng_fast/cprng_fast.c:1.20
--- src/sys/crypto/cprng_fast/cprng_fast.c:1.19	Sat Aug  5 11:39:18 2023
+++ src/sys/crypto/cprng_fast/cprng_fast.c	Tue Oct 15 17:34:06 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: cprng_fast.c,v 1.19 2023/08/05 11:39:18 riastradh Exp $	*/
+/*	$NetBSD: cprng_fast.c,v 1.20 2024/10/15 17:34:06 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cprng_fast.c,v 1.19 2023/08/05 11:39:18 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cprng_fast.c,v 1.20 2024/10/15 17:34:06 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -39,9 +39,9 @@ __KERNEL_RCSID(0, "$NetBSD: cprng_fast.c
 #include <sys/cpu.h>
 #include <sys/entropy.h>
 #include <sys/evcnt.h>
+#include <sys/intr.h>
 #include <sys/kmem.h>
 #include <sys/percpu.h>
-#include <sys/pserialize.h>
 
 #include <crypto/chacha/chacha.h>
 
@@ -58,7 +58,8 @@ struct cprng_fast {
 };
 
 static void	cprng_fast_init_cpu(void *, void *, struct cpu_info *);
-static void	cprng_fast_reseed(struct cprng_fast **, unsigned);
+static void	cprng_fast_schedule_reseed(struct cprng_fast *);
+static void	cprng_fast_intr(void *);
 
 static void	cprng_fast_seed(struct cprng_fast *, const void *);
 static void	cprng_fast_buf(struct cprng_fast *, void *, unsigned);
@@ -67,6 +68,7 @@ static void	cprng_fast_buf_short(void *,
 static void	cprng_fast_buf_long(void *, size_t);
 
 static percpu_t	*cprng_fast_percpu	__read_mostly;
+static void	*cprng_fast_softint	__read_mostly;
 
 void
 cprng_fast_init(void)
@@ -74,14 +76,20 @@ cprng_fast_init(void)
 
 	cprng_fast_percpu = percpu_create(sizeof(struct cprng_fast),
 	    cprng_fast_init_cpu, NULL, NULL);
+	cprng_fast_softint = softint_establish(SOFTINT_SERIAL|SOFTINT_MPSAFE,
+	    &cprng_fast_intr, NULL);
 }
 
 static void
 cprng_fast_init_cpu(void *p, void *arg __unused, struct cpu_info *ci)
 {
 	struct cprng_fast *const cprng = p;
+	uint8_t seed[CPRNG_FAST_SEED_BYTES];
 
-	cprng->epoch = 0;
+	cprng->epoch = entropy_epoch();
+	cprng_strong(kern_cprng, seed, sizeof seed, 0);
+	cprng_fast_seed(cprng, seed);
+	(void)explicit_memset(seed, 0, sizeof seed);
 
 	cprng->reseed_evcnt = kmem_alloc(sizeof(*cprng->reseed_evcnt),
 	    KM_SLEEP);
@@ -93,21 +101,13 @@ static int
 cprng_fast_get(struct cprng_fast **cprngp)
 {
 	struct cprng_fast *cprng;
-	unsigned epoch;
 	int s;
 
-	KASSERT(!cpu_intr_p());
-	KASSERT(pserialize_not_in_read_section());
-
 	*cprngp = cprng = percpu_getref(cprng_fast_percpu);
-	s = splsoftserial();
+	s = splvm();
 
-	epoch = entropy_epoch();
-	if (__predict_false(cprng->epoch != epoch)) {
-		splx(s);
-		cprng_fast_reseed(cprngp, epoch);
-		s = splsoftserial();
-	}
+	if (__predict_false(cprng->epoch != entropy_epoch()))
+		cprng_fast_schedule_reseed(cprng);
 
 	return s;
 }
@@ -123,31 +123,29 @@ cprng_fast_put(struct cprng_fast *cprng,
 }
 
 static void
-cprng_fast_reseed(struct cprng_fast **cprngp, unsigned epoch)
+cprng_fast_schedule_reseed(struct cprng_fast *cprng __unused)
 {
+
+	softint_schedule(cprng_fast_softint);
+}
+
+static void
+cprng_fast_intr(void *cookie __unused)
+{
+	unsigned epoch = entropy_epoch();
 	struct cprng_fast *cprng;
 	uint8_t seed[CPRNG_FAST_SEED_BYTES];
 	int s;
 
-	/*
-	 * Drop the percpu(9) reference to extract a fresh seed from
-	 * the entropy pool.  cprng_strong may sleep on an adaptive
-	 * lock, which invalidates our percpu(9) reference.
-	 *
-	 * This may race with reseeding in another thread, which is no
-	 * big deal -- worst case, we rewind the entropy epoch here and
-	 * cause the next caller to reseed again, and in the end we
-	 * just reseed a couple more times than necessary.
-	 */
-	percpu_putref(cprng_fast_percpu);
 	cprng_strong(kern_cprng, seed, sizeof(seed), 0);
-	*cprngp = cprng = percpu_getref(cprng_fast_percpu);
 
-	s = splsoftserial();
+	cprng = percpu_getref(cprng_fast_percpu);
+	s = splvm();
 	cprng_fast_seed(cprng, seed);
 	cprng->epoch = epoch;
 	cprng->reseed_evcnt->ev_count++;
 	splx(s);
+	percpu_putref(cprng_fast_percpu);
 
 	explicit_memset(seed, 0, sizeof(seed));
 }

Reply via email to