From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: sysctl int atomic
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: tech@openbsd.org
Date: Tue, 30 Apr 2024 13:08:55 +0200

On Fri, Apr 26, 2024 at 10:49:16PM +0200, Mark Kettenis wrote:
> > Date: Fri, 26 Apr 2024 18:13:03 +0200
> > From: Alexander Bluhm <bluhm@openbsd.org>
> > 
> > Hi,
> > 
> > Interger load and store are atomic, so it is not necessary to hold
> > a lock when accessing them from sysctl(2) system call.  I added
> > some read and write once macros and memory barriers to the sysctl
> > implementation.  The sysctl_lock will keep before -> after output
> > of sysctl(8) consistent.
> > 
> > Most of the diff is mechanical.  sysctl_rdint() has a pointer
> > parameter and all callers have to adapt.  If we want to go this
> > way, I will compile test more architectures.
> > 
> > The sysctl_int_...() functions don't call each other anymore, but
> > are kept very simialar.  When comparing you see the differences.
> > 
> > After this diff has been commited, the code that uses the sysctl
> > variables can be checked if lockless accces is possible.  Then a
> > bunch of net lock in sysctl(2) can go away.
> > 
> > ok?
> 
> Sorry, but I don't think this is sensible.  You've added a
> membar_consumer() in sysctl_rdint(), but where is the matching
> membar_producer()?  Or in other words what are you ordering against?

You are right, the membars are not necessary.  I removed them.

> I suspect in most cases, no ordering is actually required.  And in the
> few cases where it might matter, you're probably better off making
> sure a consistent value is passed to sysctl_rdint() by using the
> appropriate memory barriers, atomic operations or mutex in the caller.

Atomic read or write operations on intergers cost nothing.  They
are basically a volatile cast to avoid compiler optimiations.
Presumably the compiler does not optimize anything, but I want to
be sure.

Usually I use READ_ONCE and WRITE_ONCE to mark that access is MP
safe.  And interger sysctl is MP safe with this diff.  If more
locking around sysctl_int_...() is needed, it can be kept.  But all
the other locks can be removed step by step in following diffs.

I prefer to make the underlying functions MP safe instead of caring
about all the callers.

ok?

bluhm

Index: arch/alpha/alpha/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/alpha/alpha/machdep.c,v
diff -u -p -r1.203 machdep.c
--- arch/alpha/alpha/machdep.c	11 Apr 2023 00:45:06 -0000	1.203
+++ arch/alpha/alpha/machdep.c	23 Apr 2024 01:12:41 -0000
@@ -1506,6 +1506,9 @@ cpu_sysctl(int *name, u_int namelen, voi
     size_t newlen, struct proc *p)
 {
 	dev_t consdev;
+#ifndef APERTURE
+	int val;
+#endif
 #if NIOASIC > 0
 	int oldval, ret;
 #endif
@@ -1546,7 +1549,8 @@ cpu_sysctl(int *name, u_int namelen, voi
                         return (sysctl_int(oldp, oldlenp, newp, newlen,
                             &allowaperture));
 #else
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #endif
 #if NIOASIC > 0
 	case CPU_LED_BLINK:
Index: arch/alpha/pci/pci_machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/alpha/pci/pci_machdep.c,v
diff -u -p -r1.21 pci_machdep.c
--- arch/alpha/pci/pci_machdep.c	8 Sep 2017 05:36:51 -0000	1.21
+++ arch/alpha/pci/pci_machdep.c	23 Apr 2024 01:13:13 -0000
@@ -122,7 +122,7 @@ alpha_sysctl_chipset(int *name, u_int na
 		    alpha_pci_chipset->pc_name));
 	case CPU_CHIPSET_BWX:
 		return (sysctl_rdint(where, sizep, NULL,
-		    alpha_pci_chipset->pc_bwx));
+		    &alpha_pci_chipset->pc_bwx));
 	case CPU_CHIPSET_MEM:
 		return (sysctl_rdquad(where, sizep, NULL,
 		    alpha_pci_chipset->pc_mem));
Index: arch/amd64/amd64/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/amd64/amd64/machdep.c,v
diff -u -p -r1.293 machdep.c
--- arch/amd64/amd64/machdep.c	29 Apr 2024 00:29:48 -0000	1.293
+++ arch/amd64/amd64/machdep.c	30 Apr 2024 09:40:55 -0000
@@ -466,7 +466,7 @@ bios_sysctl(int *name, u_int namelen, vo
 		if ((pdi = bios_getdiskinfo(bootdev)) == NULL)
 			return ENXIO;
 		biosdev = pdi->bios_number;
-		return sysctl_rdint(oldp, oldlenp, newp, biosdev);
+		return sysctl_rdint(oldp, oldlenp, newp, &biosdev);
 	case BIOS_DISKINFO:
 		if (namelen != 2)
 			return ENOTDIR;
@@ -474,7 +474,7 @@ bios_sysctl(int *name, u_int namelen, vo
 			return ENXIO;
 		return sysctl_rdstruct(oldp, oldlenp, newp, pdi, sizeof(*pdi));
 	case BIOS_CKSUMLEN:
-		return sysctl_rdint(oldp, oldlenp, newp, bios_cksumlen);
+		return sysctl_rdint(oldp, oldlenp, newp, &bios_cksumlen);
 	default:
 		return EOPNOTSUPP;
 	}
@@ -502,6 +502,9 @@ int
 cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
+#ifndef APERTURE
+	int val;
+#endif
 	extern uint64_t tsc_frequency;
 	dev_t consdev;
 	dev_t dev;
@@ -540,7 +543,8 @@ cpu_sysctl(int *name, u_int namelen, voi
 			return (sysctl_int(oldp, oldlenp, newp, newlen,
 			    &allowaperture));
 #else
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #endif
 #if NPCKBC > 0 && NUKBD > 0
 	case CPU_FORCEUKBD:
@@ -548,7 +552,7 @@ cpu_sysctl(int *name, u_int namelen, voi
 		int error;
 
 		if (forceukbd)
-			return (sysctl_rdint(oldp, oldlenp, newp, forceukbd));
+			return (sysctl_rdint(oldp, oldlenp, newp, &forceukbd));
 
 		error = sysctl_int(oldp, oldlenp, newp, newlen, &forceukbd);
 		if (forceukbd)
Index: arch/i386/i386/bios.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/bios.c,v
diff -u -p -r1.129 bios.c
--- arch/i386/i386/bios.c	15 Mar 2023 08:20:52 -0000	1.129
+++ arch/i386/i386/bios.c	30 Apr 2024 09:40:55 -0000
@@ -783,7 +783,7 @@ bios_sysctl(int *name, u_int namelen, vo
 		if ((pdi = bios_getdiskinfo(bootdev)) == NULL)
 			return ENXIO;
 		biosdev = pdi->bios_number;
-		return sysctl_rdint(oldp, oldlenp, newp, biosdev);
+		return sysctl_rdint(oldp, oldlenp, newp, &biosdev);
 	case BIOS_DISKINFO:
 		if (namelen != 2)
 			return ENOTDIR;
@@ -791,7 +791,7 @@ bios_sysctl(int *name, u_int namelen, vo
 			return ENXIO;
 		return sysctl_rdstruct(oldp, oldlenp, newp, pdi, sizeof(*pdi));
 	case BIOS_CKSUMLEN:
-		return sysctl_rdint(oldp, oldlenp, newp, bios_cksumlen);
+		return sysctl_rdint(oldp, oldlenp, newp, &bios_cksumlen);
 	default:
 		return EOPNOTSUPP;
 	}
Index: arch/i386/i386/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/i386/i386/machdep.c,v
diff -u -p -r1.670 machdep.c
--- arch/i386/i386/machdep.c	29 Apr 2024 00:29:48 -0000	1.670
+++ arch/i386/i386/machdep.c	30 Apr 2024 10:53:05 -0000
@@ -3495,6 +3495,7 @@ int
 cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
+	int val;
 	dev_t dev;
 
 	switch (name[0]) {
@@ -3531,12 +3532,14 @@ cpu_sysctl(int *name, u_int namelen, voi
 			return (sysctl_int(oldp, oldlenp, newp, newlen,
 			    &allowaperture));
 #else
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #endif
 	case CPU_CPUVENDOR:
 		return (sysctl_rdstring(oldp, oldlenp, newp, cpu_vendor));
 	case CPU_CPUFEATURE:
-		return (sysctl_rdint(oldp, oldlenp, newp, curcpu()->ci_feature_flags));
+		val = curcpu()->ci_feature_flags;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case CPU_KBDRESET:
 		return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen,
 		    &kbd_reset));
@@ -3546,7 +3549,7 @@ cpu_sysctl(int *name, u_int namelen, voi
 		int error;
 
 		if (forceukbd)
-			return (sysctl_rdint(oldp, oldlenp, newp, forceukbd));
+			return (sysctl_rdint(oldp, oldlenp, newp, &forceukbd));
 
 		error = sysctl_int(oldp, oldlenp, newp, newlen, &forceukbd);
 		if (forceukbd)
Index: arch/luna88k/luna88k/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/luna88k/luna88k/machdep.c,v
diff -u -p -r1.144 machdep.c
--- arch/luna88k/luna88k/machdep.c	24 Oct 2023 13:20:10 -0000	1.144
+++ arch/luna88k/luna88k/machdep.c	23 Apr 2024 01:18:56 -0000
@@ -940,7 +940,7 @@ cpu_sysctl(int *name, u_int namelen, voi
 		return (sysctl_rdstruct(oldp, oldlenp, newp, &consdev,
 		    sizeof consdev));
 	case CPU_CPUTYPE:
-		return (sysctl_rdint(oldp, oldlenp, newp, cputyp));
+		return (sysctl_rdint(oldp, oldlenp, newp, &cputyp));
 	default:
 		return (EOPNOTSUPP);
 	}
Index: arch/macppc/macppc/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/macppc/macppc/machdep.c,v
diff -u -p -r1.200 machdep.c
--- arch/macppc/macppc/machdep.c	24 Oct 2023 13:20:10 -0000	1.200
+++ arch/macppc/macppc/machdep.c	26 Apr 2024 15:57:33 -0000
@@ -544,6 +544,10 @@ int
 cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
+#ifndef APERTURE
+	int val;
+#endif
+
 	/* all sysctl names at this level are terminal */
 	if (namelen != 1)
 		return ENOTDIR;
@@ -557,7 +561,8 @@ cpu_sysctl(int *name, u_int namelen, voi
 			return (sysctl_int(oldp, oldlenp, newp, newlen,
 			    &allowaperture));
 #else
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #endif
 	default:
 		return (sysctl_bounded_arr(cpuctl_vars, nitems(cpuctl_vars),
Index: arch/powerpc64/powerpc64/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/powerpc64/powerpc64/machdep.c,v
diff -u -p -r1.74 machdep.c
--- arch/powerpc64/powerpc64/machdep.c	7 Jan 2023 17:29:37 -0000	1.74
+++ arch/powerpc64/powerpc64/machdep.c	23 Apr 2024 01:19:41 -0000
@@ -1095,7 +1095,7 @@ cpu_sysctl(int *name, u_int namelen, voi
 
 	switch (name[0]) {
 	case CPU_ALTIVEC:
-		return (sysctl_rdint(oldp, oldlenp, newp, altivec));
+		return (sysctl_rdint(oldp, oldlenp, newp, &altivec));
 	default:
 		return EOPNOTSUPP;
 	}
Index: arch/sparc64/sparc64/machdep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/arch/sparc64/sparc64/machdep.c,v
diff -u -p -r1.216 machdep.c
--- arch/sparc64/sparc64/machdep.c	29 Mar 2024 21:29:34 -0000	1.216
+++ arch/sparc64/sparc64/machdep.c	23 Apr 2024 01:21:04 -0000
@@ -341,6 +341,9 @@ int
 cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
+#ifndef APERTURE
+	int val;
+#endif
 	int oldval, ret;
 
 	/* all sysctl names at this level are terminal */
@@ -367,12 +370,13 @@ cpu_sysctl(int *name, u_int namelen, voi
 			return (sysctl_int(oldp, oldlenp, newp, newlen,
 			    &allowaperture));
 #else
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #endif
 	case CPU_CPUTYPE:
-		return (sysctl_rdint(oldp, oldlenp, newp, cputyp));
+		return (sysctl_rdint(oldp, oldlenp, newp, &cputyp));
 	case CPU_CECCERRORS:
-		return (sysctl_rdint(oldp, oldlenp, newp, ceccerrs));
+		return (sysctl_rdint(oldp, oldlenp, newp, &ceccerrs));
 	case CPU_CECCLAST:
 		return (sysctl_rdquad(oldp, oldlenp, newp, cecclast));
 	default:
Index: ddb/db_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/ddb/db_usrreq.c,v
diff -u -p -r1.22 db_usrreq.c
--- ddb/db_usrreq.c	9 Jan 2021 20:58:12 -0000	1.22
+++ ddb/db_usrreq.c	30 Apr 2024 09:40:55 -0000
@@ -48,6 +48,8 @@ int
 ddb_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
+	int val;
+
 	/* All sysctl names at this level are terminal. */
 	if (namelen != 1)
 		return (ENOTDIR);
@@ -83,7 +85,8 @@ ddb_sysctl(int *name, u_int namelen, voi
 			} else
 				return (ENODEV);
 		}
-		return (sysctl_rdint(oldp, oldlenp, newp, 0));
+		val = 0;
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 #if defined(DDBPROF)
 	case DBCTL_PROFILE:
 		if (securelevel > 0)
Index: kern/kern_sysctl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
diff -u -p -r1.427 kern_sysctl.c
--- kern/kern_sysctl.c	12 Apr 2024 16:07:09 -0000	1.427
+++ kern/kern_sysctl.c	30 Apr 2024 10:53:05 -0000
@@ -157,6 +157,7 @@ int (*cpu_cpuspeed)(int *);
 /*
  * Lock to avoid too many processes vslocking a large amount of memory
  * at the same time.
+ * Also provide consistent transitions from before to after values.
  */
 struct rwlock sysctl_lock = RWLOCK_INITIALIZER("sysctllk");
 struct rwlock sysctl_disklock = RWLOCK_INITIALIZER("sysctldlk");
@@ -448,7 +449,7 @@ int
 kern_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen, struct proc *p)
 {
-	int error, level, inthostid, stackgap;
+	int val, error, level, inthostid, stackgap;
 	dev_t dev;
 	extern int pool_debug;
 
@@ -467,8 +468,9 @@ kern_sysctl(int *name, u_int namelen, vo
 		return (sysctl_rdstring(oldp, oldlenp, newp, osversion));
 	case KERN_VERSION:
 		return (sysctl_rdstring(oldp, oldlenp, newp, version));
-	case KERN_NUMVNODES:  /* XXX numvnodes is a long */
-		return (sysctl_rdint(oldp, oldlenp, newp, numvnodes));
+	case KERN_NUMVNODES:
+		val = numvnodes;	/* XXX numvnodes is a long */
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case KERN_SECURELVL:
 		level = securelevel;
 		if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &level)) ||
@@ -543,7 +545,8 @@ kern_sysctl(int *name, u_int namelen, vo
 		 */
 		if (!mp || mp->msg_magic != MSG_MAGIC)
 			return (ENXIO);
-		return (sysctl_rdint(oldp, oldlenp, newp, mp->msg_bufs));
+		val = mp->msg_bufs;	/* XXX msg_bufs is a long */
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	}
 	case KERN_CONSBUF:
 		if ((error = suser(p)))
@@ -681,7 +684,7 @@ hw_sysctl(int *name, u_int namelen, void
     size_t newlen, struct proc *p)
 {
 	extern char machine[], cpu_model[];
-	int err, cpuspeed;
+	int val, err, cpuspeed;
 
 	/*
 	 * all sysctl names at this level except sensors and battery
@@ -696,13 +699,14 @@ hw_sysctl(int *name, u_int namelen, void
 	case HW_MODEL:
 		return (sysctl_rdstring(oldp, oldlenp, newp, cpu_model));
 	case HW_NCPUONLINE:
-		return (sysctl_rdint(oldp, oldlenp, newp,
-		    sysctl_hwncpuonline()));
+		val = sysctl_hwncpuonline();
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case HW_PHYSMEM:
-		return (sysctl_rdint(oldp, oldlenp, newp, ptoa(physmem)));
+		val = ptoa(physmem);	/* XXX unsigned long */
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case HW_USERMEM:
-		return (sysctl_rdint(oldp, oldlenp, newp,
-		    ptoa(physmem - uvmexp.wired)));
+		val = ptoa(physmem - uvmexp.wired);  /* XXX unsigned long */
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case HW_DISKNAMES:
 		err = sysctl_diskinit(0, p);
 		if (err)
@@ -724,7 +728,7 @@ hw_sysctl(int *name, u_int namelen, void
 		err = cpu_cpuspeed(&cpuspeed);
 		if (err)
 			return err;
-		return (sysctl_rdint(oldp, oldlenp, newp, cpuspeed));
+		return (sysctl_rdint(oldp, oldlenp, newp, &cpuspeed));
 #ifndef	SMALL_KERNEL
 	case HW_SENSORS:
 		return (sysctl_sensors(name + 1, namelen - 1, oldp, oldlenp,
@@ -933,17 +937,32 @@ int
 sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
     int *valp)
 {
-	unsigned int oval = *valp, val = *valp;
+	unsigned int oldval, newval;
 	int error;
 
-	if (newp == NULL)
-		return (sysctl_rdint(oldp, oldlenp, newp, val));
+	if (oldp && *oldlenp < sizeof(int))
+		return (ENOMEM);
+	if (newp && newlen != sizeof(int))
+		return (EINVAL);
+	*oldlenp = sizeof(int);
 
-	if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
-		return (error);
-	if (val > oval)
-		return (EPERM);		/* do not allow raising */
-	*(unsigned int *)valp = val;
+	/* copyin() may sleep, call it first */
+	if (newp) {
+		if ((error = copyin(newp, &newval, sizeof(int))))
+			return (error);
+	}
+	oldval = READ_ONCE(*valp);
+	if (newp) {
+		/* do not allow raising */
+		if (newval > oldval)
+			return (EPERM);
+	}
+	if (oldp) {
+		if ((error = copyout(&oldval, oldp, sizeof(int))))
+			return (error);
+	}
+	if (newp)
+		WRITE_ONCE(*valp, newval);
 	return (0);
 }
 
@@ -954,26 +973,37 @@ sysctl_int_lower(void *oldp, size_t *old
 int
 sysctl_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int *valp)
 {
-	int error = 0;
+	int oldval, newval;
+	int error;
 
 	if (oldp && *oldlenp < sizeof(int))
 		return (ENOMEM);
 	if (newp && newlen != sizeof(int))
 		return (EINVAL);
 	*oldlenp = sizeof(int);
-	if (oldp)
-		error = copyout(valp, oldp, sizeof(int));
-	if (error == 0 && newp)
-		error = copyin(newp, valp, sizeof(int));
-	return (error);
+
+	/* copyin() may sleep, call it first */
+	if (newp) {
+		if ((error = copyin(newp, &newval, sizeof(int))))
+			return (error);
+	}
+	if (oldp) {
+		oldval = READ_ONCE(*valp);
+		if ((error = copyout(&oldval, oldp, sizeof(int))))
+			return (error);
+	}
+	if (newp)
+		WRITE_ONCE(*valp, newval);
+	return (0);
 }
 
 /*
  * As above, but read-only.
  */
 int
-sysctl_rdint(void *oldp, size_t *oldlenp, void *newp, int val)
+sysctl_rdint(void *oldp, size_t *oldlenp, void *newp, const int *valp)
 {
+	int oldval;
 	int error = 0;
 
 	if (oldp && *oldlenp < sizeof(int))
@@ -981,8 +1011,11 @@ sysctl_rdint(void *oldp, size_t *oldlenp
 	if (newp)
 		return (EPERM);
 	*oldlenp = sizeof(int);
-	if (oldp)
-		error = copyout((caddr_t)&val, oldp, sizeof(int));
+
+	if (oldp) {
+		oldval = READ_ONCE(*valp);
+		error = copyout(&oldval, oldp, sizeof(int));
+	}
 	return (error);
 }
 
@@ -994,7 +1027,7 @@ sysctl_securelevel_int(void *oldp, size_
     int *valp)
 {
 	if (securelevel > 0)
-		return (sysctl_rdint(oldp, oldlenp, newp, *valp));
+		return (sysctl_rdint(oldp, oldlenp, newp, valp));
 	return (sysctl_int(oldp, oldlenp, newp, newlen, valp));
 }
 
@@ -1005,19 +1038,34 @@ int
 sysctl_int_bounded(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
     int *valp, int minimum, int maximum)
 {
-	int val = *valp;
+	int oldval, newval;
 	int error;
 
 	/* read only */
-	if (newp == NULL || minimum > maximum)
-		return (sysctl_rdint(oldp, oldlenp, newp, val));
+	if (minimum > maximum)
+		return (sysctl_rdint(oldp, oldlenp, newp, valp));
 
-	if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
-		return (error);
-	/* outside limits */
-	if (val < minimum || maximum < val)
+	if (oldp && *oldlenp < sizeof(int))
+		return (ENOMEM);
+	if (newp && newlen != sizeof(int))
 		return (EINVAL);
-	*valp = val;
+	*oldlenp = sizeof(int);
+
+	/* copyin() may sleep, call it first */
+	if (newp) {
+		if ((error = copyin(newp, &newval, sizeof(int))))
+			return (error);
+		/* outside limits */
+		if (newval < minimum || maximum < newval)
+			return (EINVAL);
+	}
+	if (oldp) {
+		oldval = READ_ONCE(*valp);
+		if ((error = copyout(&oldval, oldp, sizeof(int))))
+			return (error);
+	}
+	if (newp)
+		WRITE_ONCE(*valp, newval);
 	return (0);
 }
 
@@ -1923,11 +1971,11 @@ sysctl_proc_args(int *name, u_int namele
 		goto out;
 
 	if (op == KERN_PROC_NARGV) {
-		error = sysctl_rdint(oldp, oldlenp, NULL, pss.ps_nargvstr);
+		error = sysctl_rdint(oldp, oldlenp, NULL, &pss.ps_nargvstr);
 		goto out;
 	}
 	if (op == KERN_PROC_NENV) {
-		error = sysctl_rdint(oldp, oldlenp, NULL, pss.ps_nenvstr);
+		error = sysctl_rdint(oldp, oldlenp, NULL, &pss.ps_nenvstr);
 		goto out;
 	}
 
Index: kern/sched_bsd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/sched_bsd.c,v
diff -u -p -r1.91 sched_bsd.c
--- kern/sched_bsd.c	30 Mar 2024 13:33:20 -0000	1.91
+++ kern/sched_bsd.c	30 Apr 2024 09:40:55 -0000
@@ -662,7 +662,7 @@ sysctl_hwsetperf(void *oldp, size_t *old
 		return EOPNOTSUPP;
 
 	if (perfpolicy != PERFPOL_MANUAL)
-		return sysctl_rdint(oldp, oldlenp, newp, perflevel);
+		return sysctl_rdint(oldp, oldlenp, newp, &perflevel);
 
 	err = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
 	    &perflevel, 0, 100);
Index: kern/subr_evcount.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_evcount.c,v
diff -u -p -r1.16 subr_evcount.c
--- kern/subr_evcount.c	16 Sep 2023 09:33:27 -0000	1.16
+++ kern/subr_evcount.c	30 Apr 2024 09:40:55 -0000
@@ -123,7 +123,7 @@ evcount_sysctl(int *name, u_int namelen,
 
 	switch (name[0]) {
 	case KERN_INTRCNT_NUM:
-		error = sysctl_rdint(oldp, oldlenp, NULL, nintr);
+		error = sysctl_rdint(oldp, oldlenp, NULL, &nintr);
 		break;
 	case KERN_INTRCNT_CNT:
 		if (ec == NULL)
@@ -145,8 +145,7 @@ evcount_sysctl(int *name, u_int namelen,
 	case KERN_INTRCNT_VECTOR:
 		if (ec == NULL || ec->ec_data == NULL)
 			return (ENOENT);
-		error = sysctl_rdint(oldp, oldlenp, NULL,
-		    *((int *)ec->ec_data));
+		error = sysctl_rdint(oldp, oldlenp, NULL, ec->ec_data);
 		break;
 	default:
 		error = EOPNOTSUPP;
Index: kern/subr_pool.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_pool.c,v
diff -u -p -r1.236 subr_pool.c
--- kern/subr_pool.c	14 Aug 2022 01:58:28 -0000	1.236
+++ kern/subr_pool.c	30 Apr 2024 09:40:55 -0000
@@ -1473,7 +1473,7 @@ sysctl_dopool(int *name, u_int namelen, 
 	case KERN_POOL_NPOOLS:
 		if (namelen != 1)
 			return (ENOTDIR);
-		return (sysctl_rdint(oldp, oldlenp, NULL, pool_count));
+		return (sysctl_rdint(oldp, oldlenp, NULL, &pool_count));
 
 	case KERN_POOL_NAME:
 	case KERN_POOL_POOL:
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
diff -u -p -r1.290 uipc_mbuf.c
--- kern/uipc_mbuf.c	5 Mar 2024 18:52:41 -0000	1.290
+++ kern/uipc_mbuf.c	30 Apr 2024 09:40:55 -0000
@@ -1786,7 +1786,7 @@ sysctl_mq(int *name, u_int namelen, void
     void *newp, size_t newlen, struct mbuf_queue *mq)
 {
 	unsigned int maxlen;
-	int error;
+	int val, error;
 
 	/* All sysctl names at this level are terminal. */
 	if (namelen != 1)
@@ -1794,7 +1794,8 @@ sysctl_mq(int *name, u_int namelen, void
 
 	switch (name[0]) {
 	case IFQCTL_LEN:
-		return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq)));
+		val = mq_len(mq);
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	case IFQCTL_MAXLEN:
 		maxlen = mq->mq_maxlen;
 		error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
@@ -1802,7 +1803,8 @@ sysctl_mq(int *name, u_int namelen, void
 			mq_set_maxlen(mq, maxlen);
 		return (error);
 	case IFQCTL_DROPS:
-		return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));
+		val = mq_drops(mq);
+		return (sysctl_rdint(oldp, oldlenp, newp, &val));
 	default:
 		return (EOPNOTSUPP);
 	}
Index: kern/uipc_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_usrreq.c,v
diff -u -p -r1.204 uipc_usrreq.c
--- kern/uipc_usrreq.c	10 Apr 2024 12:04:41 -0000	1.204
+++ kern/uipc_usrreq.c	30 Apr 2024 09:40:55 -0000
@@ -733,7 +733,7 @@ uipc_sysctl(int *name, u_int namelen, vo
 	case NET_UNIX_DEFERRED:
 		if (namelen != 1)
 			return (ENOTDIR);
-		return sysctl_rdint(oldp, oldlenp, newp, *valp);
+		return sysctl_rdint(oldp, oldlenp, newp, valp);
 	default:
 		return (ENOPROTOOPT);
 	}
Index: kern/vfs_subr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_subr.c,v
diff -u -p -r1.319 vfs_subr.c
--- kern/vfs_subr.c	3 Feb 2024 18:51:58 -0000	1.319
+++ kern/vfs_subr.c	30 Apr 2024 09:40:55 -0000
@@ -1390,7 +1390,7 @@ vfs_sysctl(int *name, u_int namelen, voi
 
 	switch (name[1]) {
 	case VFS_MAXTYPENUM:
-		return (sysctl_rdint(oldp, oldlenp, newp, maxvfsconf));
+		return (sysctl_rdint(oldp, oldlenp, newp, &maxvfsconf));
 
 	case VFS_CONF:
 		if (namelen < 3)
Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
diff -u -p -r1.393 ip_input.c
--- netinet/ip_input.c	16 Apr 2024 12:56:39 -0000	1.393
+++ netinet/ip_input.c	30 Apr 2024 09:40:55 -0000
@@ -1741,8 +1741,7 @@ ip_sysctl(int *name, u_int namelen, void
 		return (sysctl_niq(name + 1, namelen - 1,
 		    oldp, oldlenp, newp, newlen, &arpinq));
 	case IPCTL_ARPQUEUED:
-		return (sysctl_rdint(oldp, oldlenp, newp,
-		    atomic_load_int(&la_hold_total)));
+		return (sysctl_rdint(oldp, oldlenp, newp, &la_hold_total));
 	case IPCTL_STATS:
 		return (ip_sysctl_ipstat(oldp, oldlenp, newp));
 #ifdef MROUTING
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
diff -u -p -r1.252 icmp6.c
--- netinet6/icmp6.c	21 Apr 2024 17:32:10 -0000	1.252
+++ netinet6/icmp6.c	30 Apr 2024 09:40:55 -0000
@@ -1906,8 +1906,7 @@ icmp6_sysctl(int *name, u_int namelen, v
 		break;
 
 	case ICMPV6CTL_ND6_QUEUED:
-		error = sysctl_rdint(oldp, oldlenp, newp,
-		    atomic_load_int(&ln_hold_total));
+		error = sysctl_rdint(oldp, oldlenp, newp, &ln_hold_total);
 		break;
 
 	default:
Index: sys/sysctl.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/sysctl.h,v
diff -u -p -r1.235 sysctl.h
--- sys/sysctl.h	1 Oct 2023 15:58:12 -0000	1.235
+++ sys/sysctl.h	30 Apr 2024 10:53:05 -0000
@@ -1056,7 +1056,7 @@ typedef int (sysctlfn)(int *, u_int, voi
 
 int sysctl_int_lower(void *, size_t *, void *, size_t, int *);
 int sysctl_int(void *, size_t *, void *, size_t, int *);
-int sysctl_rdint(void *, size_t *, void *, int);
+int sysctl_rdint(void *, size_t *, void *, const int *);
 int sysctl_securelevel_int(void *, size_t *, void *, size_t, int *);
 int sysctl_int_bounded(void *, size_t *, void *, size_t, int *, int, int);
 int sysctl_bounded_arr(const struct sysctl_bounded_args *, u_int,
Index: uvm/uvm_meter.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/uvm/uvm_meter.c,v
diff -u -p -r1.50 uvm_meter.c
--- uvm/uvm_meter.c	16 Sep 2023 09:33:27 -0000	1.50
+++ uvm/uvm_meter.c	30 Apr 2024 09:40:55 -0000
@@ -114,7 +114,7 @@ uvm_sysctl(int *name, u_int namelen, voi
 		    sizeof(uexp)));
 
 	case VM_NKMEMPAGES:
-		return (sysctl_rdint(oldp, oldlenp, newp, nkmempages));
+		return (sysctl_rdint(oldp, oldlenp, newp, &nkmempages));
 
 	case VM_PSSTRINGS:
 		return (sysctl_rdstruct(oldp, oldlenp, newp, &pr->ps_strings,
@@ -160,10 +160,11 @@ uvm_sysctl(int *name, u_int namelen, voi
 		return rv;
 
 	case VM_MAXSLP:
-		return (sysctl_rdint(oldp, oldlenp, newp, maxslp));
+		return (sysctl_rdint(oldp, oldlenp, newp, &maxslp));
 
 	case VM_USPACE:
-		return (sysctl_rdint(oldp, oldlenp, newp, USPACE));
+		t = USPACE;
+		return (sysctl_rdint(oldp, oldlenp, newp, &t));
 
 	case VM_MALLOC_CONF:
 		return (sysctl_string(oldp, oldlenp, newp, newlen,
Index: uvm/uvm_swap_encrypt.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/uvm/uvm_swap_encrypt.c,v
diff -u -p -r1.24 uvm_swap_encrypt.c
--- uvm/uvm_swap_encrypt.c	12 Mar 2021 14:15:49 -0000	1.24
+++ uvm/uvm_swap_encrypt.c	30 Apr 2024 09:40:55 -0000
@@ -79,9 +79,9 @@ swap_encrypt_ctl(int *name, u_int namele
 		return (0);
 	}
 	case SWPENC_CREATED:
-		return (sysctl_rdint(oldp, oldlenp, newp, uvm_swpkeyscreated));
+		return (sysctl_rdint(oldp, oldlenp, newp, &uvm_swpkeyscreated));
 	case SWPENC_DELETED:
-		return (sysctl_rdint(oldp, oldlenp, newp, uvm_swpkeysdeleted));
+		return (sysctl_rdint(oldp, oldlenp, newp, &uvm_swpkeysdeleted));
 	default:
 		return (EOPNOTSUPP);
 	}