From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: sysctl(2), video(4): unlock sysctl_video()
To: kirill@openbsd.org, mglocker@openbsd.org, tech@openbsd.org
Date: Sun, 15 Dec 2024 23:47:20 +0300

This sysctl(2) path contains only `video_record_enable', which is
atomically accessed boolean integer.

Index: sys/dev/video.c
===================================================================
RCS file: /cvs/src/sys/dev/video.c,v
diff -u -p -r1.58 video.c
--- sys/dev/video.c	15 Dec 2024 18:23:56 -0000	1.58
+++ sys/dev/video.c	15 Dec 2024 20:28:01 -0000
@@ -38,6 +38,7 @@
 
 /*
  * Locks used to protect struct members and global data
+ *	a	atomic
  *	m	sc_mtx
  */
 
@@ -93,7 +94,7 @@ struct cfdriver video_cd = {
 /*
  * Global flag to control if video recording is enabled by kern.video.record.
  */
-int video_record_enable = 0;
+int video_record_enable = 0;	/* [a] */
 
 int
 videoprobe(struct device *parent, void *match, void *aux)
@@ -234,7 +235,7 @@ videoread(dev_t dev, struct uio *uio, in
 
 	/* move no more than 1 frame to userland, as per specification */
 	size = ulmin(uio->uio_resid, sc->sc_fsize);
-	if (!video_record_enable)
+	if (!atomic_load_int(&video_record_enable))
 		bzero(sc->sc_fbuffer, size);
 	error = uiomove(sc->sc_fbuffer, size, uio);
 	if (error)
@@ -371,7 +372,7 @@ videoioctl(dev_t dev, u_long cmd, caddr_
 		}
 		error = (sc->hw_if->dqbuf)(sc->hw_hdl,
 		    (struct v4l2_buffer *)data);
-		if (!video_record_enable)
+		if (!atomic_load_int(&video_record_enable))
 			bzero(sc->sc_fbuffer_mmap + vb->m.offset, vb->length);
 		mtx_enter(&sc->sc_mtx);
 		sc->sc_frames_ready--;
Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
diff -u -p -r1.457 kern_sysctl.c
--- sys/kern/kern_sysctl.c	15 Dec 2024 18:25:12 -0000	1.457
+++ sys/kern/kern_sysctl.c	15 Dec 2024 20:28:01 -0000
@@ -161,6 +161,10 @@ void fill_kproc(struct process *, struct
 
 int kern_sysctl_locked(int *, u_int, void *, size_t *, void *, size_t,
 	struct proc *);
+int kern_sysctl_dirs(int, int *, u_int, void *, size_t *, void *,
+	size_t, struct proc *);
+int kern_sysctl_dirs_locked(int, int *, u_int, void *, size_t *, void *,
+	size_t, struct proc *);
 int hw_sysctl_locked(int *, u_int, void *, size_t *,void *, size_t,
 	struct proc *);
 
@@ -395,6 +399,38 @@ int
 kern_sysctl_dirs(int top_name, int *name, u_int namelen,
     void *oldp, size_t *oldlenp, void *newp, size_t newlen, struct proc *p)
 {
+	size_t savelen;
+	int error;
+
+	switch (top_name) {
+#if NAUDIO > 0
+	case KERN_AUDIO:
+		return (sysctl_audio(name, namelen, oldp, oldlenp,
+		    newp, newlen));
+#endif
+#if NVIDEO > 0
+	case KERN_VIDEO:
+		return (sysctl_video(name, namelen, oldp, oldlenp,
+		    newp, newlen));
+#endif
+	default:
+		break;
+	}
+
+	savelen = *oldlenp;
+	if ((error = sysctl_vslock(oldp, savelen)))
+		return (error);
+	error = kern_sysctl_dirs_locked(top_name, name, namelen,
+	    oldp, oldlenp, newp, newlen, p);
+	sysctl_vsunlock(oldp, savelen);
+
+	return (error);
+}
+
+int
+kern_sysctl_dirs_locked(int top_name, int *name, u_int namelen,
+    void *oldp, size_t *oldlenp, void *newp, size_t newlen, struct proc *p)
+{
 	switch (top_name) {
 #ifndef SMALL_KERNEL
 	case KERN_PROC:
@@ -462,11 +498,6 @@ kern_sysctl_dirs(int top_name, int *name
 		return witness_sysctl(name, namelen, oldp, oldlenp,
 		    newp, newlen);
 #endif
-#if NVIDEO > 0
-	case KERN_VIDEO:
-		return (sysctl_video(name, namelen, oldp, oldlenp,
-		    newp, newlen));
-#endif
 	case KERN_CPUSTATS:
 		return (sysctl_cpustats(name, namelen, oldp, oldlenp,
 		    newp, newlen));
@@ -489,25 +520,9 @@ kern_sysctl(int *name, u_int namelen, vo
 	size_t savelen;
 
 	/* dispatch the non-terminal nodes first */
-	if (namelen != 1) {
-		switch (name[0]) {
-#if NAUDIO > 0
-		case KERN_AUDIO:
-			return (sysctl_audio(name + 1, namelen - 1,
-			    oldp, oldlenp, newp, newlen));
-#endif
-		default:
-			break;
-		}
-
-		savelen = *oldlenp;
-		if ((error = sysctl_vslock(oldp, savelen)))
-			return (error);
-		error = kern_sysctl_dirs(name[0], name + 1, namelen - 1,
-		    oldp, oldlenp, newp, newlen, p);
-		sysctl_vsunlock(oldp, savelen);
-		return (error);
-	}
+	if (namelen != 1)
+		return (kern_sysctl_dirs(name[0], name + 1, namelen - 1,
+		    oldp, oldlenp, newp, newlen, p));
 
 	switch (name[0]) {
 	case KERN_ALLOWKMEM: