From 278923c07fda6048b6fcd284d05cda35cc047737 Mon Sep 17 00:00:00 2001
From: bluhm <bluhm@openbsd.org>
Date: Wed, 22 Dec 2021 22:20:13 +0000
Subject: [PATCH] While malloc sleeps, the disk list could change during
 sysctl.  Then allocated memory could be too short for the list of disks. 
 Retry allocating enough space until it did not change. The disk list and duid
 memory are protected by kernel lock.  Use asserts to mark this explicitly.
 Reported-by: syzbot+807423f6868bbfb836bc@syzkaller.appspotmail.com OK anton@
 mpi@

---
 sys/kern/kern_sysctl.c | 31 ++++++++++++++++++++++---------
 sys/kern/subr_disk.c   |  7 ++++++-
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index aacba022fd7..f3d3be011ed 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sysctl.c,v 1.396 2021/10/30 23:24:48 deraadt Exp $	*/
+/*	$OpenBSD: kern_sysctl.c,v 1.397 2021/12/22 22:20:13 bluhm Exp $	*/
 /*	$NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $	*/
 
 /*-
@@ -2132,12 +2132,19 @@ sysctl_diskinit(int update, struct proc *p)
 	struct diskstats *sdk;
 	struct disk *dk;
 	const char *duid;
-	int i, tlen, l;
+	int i, changed = 0;
+
+	KERNEL_ASSERT_LOCKED();
 
 	if ((i = rw_enter(&sysctl_disklock, RW_WRITE|RW_INTR)) != 0)
 		return i;
 
-	if (disk_change) {
+	/* Run in a loop, disks may change while malloc sleeps. */
+	while (disk_change) {
+		int tlen;
+
+		disk_change = 0;
+
 		for (dk = TAILQ_FIRST(&disklist), tlen = 0; dk;
 		    dk = TAILQ_NEXT(dk, dk_link)) {
 			if (dk->dk_name)
@@ -2146,10 +2153,12 @@ sysctl_diskinit(int update, struct proc *p)
 		}
 		tlen++;
 
-		if (disknames)
-			free(disknames, M_SYSCTL, disknameslen);
-		if (diskstats)
-			free(diskstats, M_SYSCTL, diskstatslen);
+		/*
+		 * The sysctl_disklock ensures that no other process can
+		 * allocate disknames and diskstats while our malloc sleeps.
+		 */
+		free(disknames, M_SYSCTL, disknameslen);
+		free(diskstats, M_SYSCTL, diskstatslen);
 		diskstats = NULL;
 		disknames = NULL;
 		diskstats = mallocarray(disk_count, sizeof(struct diskstats),
@@ -2158,13 +2167,18 @@ sysctl_diskinit(int update, struct proc *p)
 		disknames = malloc(tlen, M_SYSCTL, M_WAITOK|M_ZERO);
 		disknameslen = tlen;
 		disknames[0] = '\0';
+		changed = 1;
+	}
+
+	if (changed) {
+		int l;
 
 		for (dk = TAILQ_FIRST(&disklist), i = 0, l = 0; dk;
 		    dk = TAILQ_NEXT(dk, dk_link), i++) {
 			duid = NULL;
 			if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
 				duid = duid_format(dk->dk_label->d_uid);
-			snprintf(disknames + l, tlen - l, "%s:%s,",
+			snprintf(disknames + l, disknameslen - l, "%s:%s,",
 			    dk->dk_name ? dk->dk_name : "",
 			    duid ? duid : "");
 			l += strlen(disknames + l);
@@ -2187,7 +2201,6 @@ sysctl_diskinit(int update, struct proc *p)
 		/* Eliminate trailing comma */
 		if (l != 0)
 			disknames[l - 1] = '\0';
-		disk_change = 0;
 	} else if (update) {
 		/* Just update, number of drives hasn't changed */
 		for (dk = TAILQ_FIRST(&disklist), i = 0; dk;
diff --git a/sys/kern/subr_disk.c b/sys/kern/subr_disk.c
index ecff6940caa..9673efe365f 100644
--- a/sys/kern/subr_disk.c
+++ b/sys/kern/subr_disk.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: subr_disk.c,v 1.246 2021/10/24 00:02:25 jsg Exp $	*/
+/*	$OpenBSD: subr_disk.c,v 1.247 2021/12/22 22:20:13 bluhm Exp $	*/
 /*	$NetBSD: subr_disk.c,v 1.17 1996/03/16 23:17:08 christos Exp $	*/
 
 /*
@@ -1045,6 +1045,8 @@ disk_attach(struct device *dv, struct disk *diskp)
 {
 	int majdev;
 
+	KERNEL_ASSERT_LOCKED();
+
 	if (!ISSET(diskp->dk_flags, DKF_CONSTRUCTED))
 		disk_construct(diskp);
 
@@ -1130,6 +1132,7 @@ done:
 void
 disk_detach(struct disk *diskp)
 {
+	KERNEL_ASSERT_LOCKED();
 
 	if (softraid_disk_attach)
 		softraid_disk_attach(diskp, -1);
@@ -1846,6 +1849,8 @@ duid_format(u_char *duid)
 {
 	static char duid_str[17];
 
+	KERNEL_ASSERT_LOCKED();
+
 	snprintf(duid_str, sizeof(duid_str),
 	    "%02x%02x%02x%02x%02x%02x%02x%02x",
 	    duid[0], duid[1], duid[2], duid[3],
-- 
2.20.1