Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
syzkaller sysctl hw.disknames
To:
tech@openbsd.org
Date:
Tue, 24 Sep 2024 00:13:39 +0200

Download raw body.

Thread
  • Alexander Bluhm:

    syzkaller sysctl hw.disknames

Hi,

syzkaller has found a race in sysctl hw.disknames .

https://syzkaller.appspot.com/bug?extid=36e1f3b306f721f90c72

When mallocarray(9) sleeps, disk_count can change, and diskstatslen
gets inconsistent.  Then free(9) panics.

Diff below should fix this.

ok?

bluhm

Index: kern/kern_sysctl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
diff -u -p -r1.446 kern_sysctl.c
--- kern/kern_sysctl.c	29 Aug 2024 10:44:40 -0000	1.446
+++ kern/kern_sysctl.c	23 Sep 2024 19:26:30 -0000
@@ -2491,7 +2491,7 @@ sysctl_diskinit(int update, struct proc 
 
 	/* Run in a loop, disks may change while malloc sleeps. */
 	while (disk_change) {
-		int tlen;
+		int tlen, count;
 
 		disk_change = 0;
 
@@ -2502,6 +2502,8 @@ sysctl_diskinit(int update, struct proc 
 			tlen += 18;	/* label uid + separators */
 		}
 		tlen++;
+		/* disk_count may change when malloc sleeps */
+		count = disk_count;
 
 		/*
 		 * The sysctl_disklock ensures that no other process can
@@ -2511,9 +2513,9 @@ sysctl_diskinit(int update, struct proc 
 		free(diskstats, M_SYSCTL, diskstatslen);
 		diskstats = NULL;
 		disknames = NULL;
-		diskstats = mallocarray(disk_count, sizeof(struct diskstats),
+		diskstats = mallocarray(count, sizeof(struct diskstats),
 		    M_SYSCTL, M_WAITOK|M_ZERO);
-		diskstatslen = disk_count * sizeof(struct diskstats);
+		diskstatslen = count * sizeof(struct diskstats);
 		disknames = malloc(tlen, M_SYSCTL, M_WAITOK|M_ZERO);
 		disknameslen = tlen;
 		disknames[0] = '\0';