Hi, syzkaller found a race in sysctl_diskinit().
https://syzkaller.appspot.com/bug?id=76838ab8f15c5f1bc22541c60c3c279314e13db0 While malloc sleeps, the disk list could change. Retry allocating enough space until it did not change. Not sure if this is the bug which syzkaller has found. But the allocated memory could be too short for the list of disks. The whole thing is protected by kernel lock. Use asserts to make it explicit. ok? bluhm Index: kern/kern_sysctl.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v retrieving revision 1.396 diff -u -p -r1.396 kern_sysctl.c --- kern/kern_sysctl.c 30 Oct 2021 23:24:48 -0000 1.396 +++ kern/kern_sysctl.c 21 Dec 2021 16:38:59 -0000 @@ -2132,12 +2132,19 @@ sysctl_diskinit(int update, struct proc 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 } 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 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 /* 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; Index: kern/subr_disk.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_disk.c,v retrieving revision 1.246 diff -u -p -r1.246 subr_disk.c --- kern/subr_disk.c 24 Oct 2021 00:02:25 -0000 1.246 +++ kern/subr_disk.c 21 Dec 2021 15:20:46 -0000 @@ -1045,6 +1045,8 @@ disk_attach(struct device *dv, struct di { 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); @@ -1845,6 +1848,8 @@ const char * 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",
