Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
sysctl: unlock KERN_CONSBUF and KERN_MSGBUF
To:
Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date:
Sat, 3 May 2025 01:55:30 +0300

Download raw body.

Thread
We only need to keep buffer's header consistent. Store it to local
variable and deliver data with two steps.

Do we really need to fill `ump' with nulls and do manually field to
field copy? msgbuf header is all longs, it wouldn't be padded.

Index: sys/kern/kern_sysctl.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.465
diff -u -p -r1.465 kern_sysctl.c
--- sys/kern/kern_sysctl.c	27 Apr 2025 00:58:55 -0000	1.465
+++ sys/kern/kern_sysctl.c	2 May 2025 22:47:32 -0000
@@ -576,8 +576,9 @@ kern_sysctl(int *name, u_int namelen, vo
 	}
 	case KERN_MSGBUFSIZE:
 	case KERN_CONSBUFSIZE: {
-		struct msgbuf *mp;
-		mp = (name[0] == KERN_MSGBUFSIZE) ? msgbufp : consbufp;
+		struct msgbuf *mp = (name[0] == KERN_MSGBUFSIZE) ?
+		    msgbufp : consbufp;
+
 		/*
 		 * deal with cases where the message buffer has
 		 * become corrupted.
@@ -586,6 +587,38 @@ kern_sysctl(int *name, u_int namelen, vo
 			return (ENXIO);
 		return (sysctl_rdint(oldp, oldlenp, newp, mp->msg_bufs));
 	}
+	case KERN_CONSBUF:
+		if ((error = suser(p)))
+			return (error);
+		/* FALLTHROUGH */
+	case KERN_MSGBUF: {
+		extern struct mutex log_mtx;
+		struct msgbuf ump, *mp = (name[0] == KERN_MSGBUF) ?
+		    msgbufp : consbufp;
+		size_t hlen = offsetof(struct msgbuf, msg_bufc), tlen;
+
+		/*
+		 * deal with cases where the message buffer has
+		 * become corrupted.
+		 */
+		if (!mp || mp->msg_magic != MSG_MAGIC)
+			return (ENXIO);
+
+		if ((tlen = hlen + mp->msg_bufs) > *oldlenp)
+			return (ENOMEM);
+
+		mtx_enter(&log_mtx);
+		memcpy(&ump, mp, hlen);
+		mtx_leave(&log_mtx);
+
+		/* copy header... */
+		if ((error = copyout(&ump, oldp, hlen)))
+			return (error);
+		/* ...and the data. */
+		error = copyout(&mp->msg_bufc, oldp + hlen, mp->msg_bufs);
+
+		return (error);
+	}
 	case KERN_TIMEOUT_STATS:
 		return (timeout_sysctl(oldp, oldlenp, newp, newlen));
 	case KERN_OSREV:
@@ -656,22 +689,6 @@ kern_sysctl_locked(int *name, u_int name
 		if (newp && !error)
 			domainnamelen = newlen;
 		return (error);
-	case KERN_CONSBUF:
-		if ((error = suser(p)))
-			return (error);
-		/* FALLTHROUGH */
-	case KERN_MSGBUF: {
-		struct msgbuf *mp;
-		mp = (name[0] == KERN_MSGBUF) ? msgbufp : consbufp;
-		/*
-		 * deal with cases where the message buffer has
-		 * become corrupted.
-		 */
-		if (!mp || mp->msg_magic != MSG_MAGIC)
-			return (ENXIO);
-		return (sysctl_rdstruct(oldp, oldlenp, newp, mp,
-		    mp->msg_bufs + offsetof(struct msgbuf, msg_bufc)));
-	}
 	case KERN_CPTIME:
 	{
 		CPU_INFO_ITERATOR cii;