From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: sysctl: unlock KERN_CPTIME{,2}
To: David Gwynne <david@gwynne.id.au>, tech@openbsd.org
Date: Wed, 4 Jun 2025 22:48:08 +0300

On Wed, Jun 04, 2025 at 10:26:32PM +0300, Vitaliy Makkoveev wrote:
> Fixed subject: Subject: sysctl: unlock KERN_CPTIME{,2}
> 
> On Wed, Jun 04, 2025 at 10:08:27PM +0300, Vitaliy Makkoveev wrote:
> > dlg@ made the CPU state statistics mp-safe, but forgot to move sysctl(2)
> > cases out of locks.
> > 

The KERN_CPUSTATS case also uses sysctl_ci_cp_time() so it is mp-safe
too.

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.475
diff -u -p -r1.475 kern_sysctl.c
--- sys/kern/kern_sysctl.c	4 Jun 2025 13:29:11 -0000	1.475
+++ sys/kern/kern_sysctl.c	4 Jun 2025 19:45:53 -0000
@@ -410,6 +410,9 @@ kern_sysctl_dirs(int top_name, int *name
 	case KERN_MALLOCSTATS:
 		return (sysctl_malloc(name, namelen, oldp, oldlenp,
 		    newp, newlen, p));
+	case KERN_CPTIME2:
+		return (sysctl_cptime2(name, namelen, oldp, oldlenp,
+		    newp, newlen));
 	case KERN_POOL:
 		return (sysctl_dopool(name, namelen, oldp, oldlenp));
 #if NAUDIO > 0
@@ -422,6 +425,9 @@ kern_sysctl_dirs(int top_name, int *name
 		return (sysctl_video(name, namelen, oldp, oldlenp,
 		    newp, newlen));
 #endif
+	case KERN_CPUSTATS:
+		return (sysctl_cpustats(name, namelen, oldp, oldlenp,
+		    newp, newlen));
 	default:
 		break;
 	}
@@ -490,9 +496,6 @@ kern_sysctl_dirs_locked(int top_name, in
 #endif
 	case KERN_TIMECOUNTER:
 		return (sysctl_tc(name, namelen, oldp, oldlenp, newp, newlen));
-	case KERN_CPTIME2:
-		return (sysctl_cptime2(name, namelen, oldp, oldlenp,
-		    newp, newlen));
 #ifdef WITNESS
 	case KERN_WITNESSWATCH:
 		return witness_sysctl_watch(oldp, oldlenp, newp, newlen);
@@ -500,9 +503,6 @@ kern_sysctl_dirs_locked(int top_name, in
 		return witness_sysctl(name, namelen, oldp, oldlenp,
 		    newp, newlen);
 #endif
-	case KERN_CPUSTATS:
-		return (sysctl_cpustats(name, namelen, oldp, oldlenp,
-		    newp, newlen));
 	case KERN_CLOCKINTR:
 		return sysctl_clockintr(name, namelen, oldp, oldlenp, newp,
 		    newlen);
@@ -632,6 +632,33 @@ kern_sysctl(int *name, u_int namelen, vo
 			return (ENXIO);
 		return (sysctl_rdint(oldp, oldlenp, newp, mp->msg_bufs));
 	}
+	case KERN_CPTIME:
+	{
+		CPU_INFO_ITERATOR cii;
+		struct cpu_info *ci;
+		long cp_time[CPUSTATES];
+		int i, n = 0;
+
+		memset(cp_time, 0, sizeof(cp_time));
+
+		CPU_INFO_FOREACH(cii, ci) {
+			uint64_t ci_cp_time[CPUSTATES];
+
+			if (!cpu_is_online(ci))
+				continue;
+
+			n++;
+			sysctl_ci_cp_time(ci, ci_cp_time);
+			for (i = 0; i < CPUSTATES; i++)
+				cp_time[i] += ci_cp_time[i];
+		}
+
+		for (i = 0; i < CPUSTATES; i++)
+			cp_time[i] /= n;
+
+		return (sysctl_rdstruct(oldp, oldlenp, newp, &cp_time,
+		    sizeof(cp_time)));
+	}
 	case KERN_POOL_DEBUG: {
 		extern int pool_debug;
 		int oldval, newval;
@@ -714,33 +741,6 @@ kern_sysctl_locked(int *name, u_int name
 		if (newp && !error)
 			domainnamelen = newlen;
 		return (error);
-	case KERN_CPTIME:
-	{
-		CPU_INFO_ITERATOR cii;
-		struct cpu_info *ci;
-		long cp_time[CPUSTATES];
-		int i, n = 0;
-
-		memset(cp_time, 0, sizeof(cp_time));
-
-		CPU_INFO_FOREACH(cii, ci) {
-			uint64_t ci_cp_time[CPUSTATES];
-
-			if (!cpu_is_online(ci))
-				continue;
-
-			n++;
-			sysctl_ci_cp_time(ci, ci_cp_time);
-			for (i = 0; i < CPUSTATES; i++)
-				cp_time[i] += ci_cp_time[i];
-		}
-
-		for (i = 0; i < CPUSTATES; i++)
-			cp_time[i] /= n;
-
-		return (sysctl_rdstruct(oldp, oldlenp, newp, &cp_time,
-		    sizeof(cp_time)));
-	}
 	case KERN_NCHSTATS:
 		return (sysctl_rdstruct(oldp, oldlenp, newp, &nchstats,
 		    sizeof(struct nchstats)));