From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: sysctl: relax netlock around ESPCTL_UDPENCAP_ENABLE and ESPCTL_UDPENCAP_PORT
To: Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date: Sat, 7 Jun 2025 04:25:58 +0300

Both `udpencap_enable' and `udpencap_port' are inetgers with the
read-write access via sysctl(2) and read-only access from the stack.
However, the recursive calls of ipsp_process_packet(),
ipsp_process_done() and (*xf_output)() make it complicated to be
atomically loads.

So, keep the sysctl modification protected by netlock, but move the
sysctl read-only access and copying out of netlock.

Index: sys/netinet/ipsec_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.217
diff -u -p -r1.217 ipsec_input.c
--- sys/netinet/ipsec_input.c	3 Jun 2025 14:49:05 -0000	1.217
+++ sys/netinet/ipsec_input.c	7 Jun 2025 01:04:28 -0000
@@ -126,10 +126,6 @@ const struct sysctl_bounded_args espctl_
 	{ESPCTL_ENABLE, &esp_enable, 0, 1},
 };
 
-const struct sysctl_bounded_args espctl_vars_locked[] = {
-	{ESPCTL_UDPENCAP_ENABLE, &udpencap_enable, 0, 1},
-	{ESPCTL_UDPENCAP_PORT, &udpencap_port, 0, 65535},
-};
 const struct sysctl_bounded_args ahctl_vars[] = {
 	{AHCTL_ENABLE, &ah_enable, 0, 1},
 };
@@ -718,8 +714,6 @@ int
 esp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen)
 {
-	int error;
-
 	/* All sysctl names at this level are terminal. */
 	if (namelen != 1)
 		return (ENOTDIR);
@@ -727,16 +721,33 @@ esp_sysctl(int *name, u_int namelen, voi
 	switch (name[0]) {
 	case ESPCTL_STATS:
 		return (esp_sysctl_espstat(oldp, oldlenp, newp));
-	case ESPCTL_ENABLE:
-		error = sysctl_bounded_arr(espctl_vars, nitems(espctl_vars),
-		    name, namelen, oldp, oldlenp, newp, newlen);
-	default:
-		NET_LOCK();
-		error = sysctl_bounded_arr(espctl_vars_locked,
-		    nitems(espctl_vars_locked),
-		    name, namelen, oldp, oldlenp, newp, newlen);
-		NET_UNLOCK();
+	case ESPCTL_UDPENCAP_ENABLE:
+	case ESPCTL_UDPENCAP_PORT: {
+		int *var, upper_bound, oval, nval, error;
+
+		if (name[0] == ESPCTL_UDPENCAP_ENABLE) {
+			var = &udpencap_enable;
+			upper_bound = 1;
+		} else {
+			var = &udpencap_port;
+			upper_bound = 65535;
+		}
+
+		oval = nval = atomic_load_int(var);
+		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
+		    &nval, 0, upper_bound);
+
+		if (error == 0 && oval != nval) {
+			NET_LOCK();
+			atomic_store_int(var, nval);
+			NET_UNLOCK();
+		}
+
 		return (error);
+	}
+	default:
+		return (sysctl_bounded_arr(espctl_vars, nitems(espctl_vars),
+		    name, namelen, oldp, oldlenp, newp, newlen));
 	}
 }
 
Index: sys/netinet/ipsec_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ipsec_output.c,v
retrieving revision 1.102
diff -u -p -r1.102 ipsec_output.c
--- sys/netinet/ipsec_output.c	3 Jun 2025 14:49:05 -0000	1.102
+++ sys/netinet/ipsec_output.c	7 Jun 2025 01:04:28 -0000
@@ -51,6 +51,11 @@
 #include <crypto/cryptodev.h>
 #include <crypto/xform.h>
 
+/*
+ * Locks used to protect data:
+ *	N	net lock
+ */
+ 
 #ifdef ENCDEBUG
 #define DPRINTF(fmt, args...)						\
 	do {								\
@@ -62,8 +67,8 @@
 	do { } while (0)
 #endif
 
-int	udpencap_enable = 1;	/* enabled by default */
-int	udpencap_port = 4500;	/* triggers decapsulation */
+int	udpencap_enable = 1;	/* [N] enabled by default */
+int	udpencap_port = 4500;	/* [N] triggers decapsulation */
 
 /*
  * Loop over a tdb chain, taking into consideration protocol tunneling. The