⬢ DragonFlyBSD Kernel Audit
DF-4116 / fix.diff
← back to finding ↓ download raw
Fix for DF-4116: guaranteed panic in sndstat_sysuninit on sound.ko unload.

sndstat.c keeps a single global sbuf whose lifecycle is tracked by
sndstat_buflen with the invariant "buflen == -1  <=>  sbuf not
allocated".  The static initializer, however, sets sndstat_buflen = 0
("allocated, empty"), while the sbuf is only actually created in
sndstat_open() via sbuf_new().  If sound.ko is unloaded before any
process has ever opened /dev/sndstat since load, sndstat_uninit()
reaches SNDSTAT_FLUSH() (buflen == 0 != -1) and calls sbuf_delete() on
the never-initialized, all-zero static sbuf:

    panic: sbuf_delete called with uninitialized or corrupt sbuf
    sbuf_delete() at sbuf_delete+0x1b
    sndstat_sysuninit() at sndstat_sysuninit+0x8c
    linker_file_unload() ...
    sys_kldunload() ...

The PID guard in sndstat_uninit() does NOT block this path: the sound
module's MOD_UNLOAD handler (sound.c sound_modevent) calls
sndstat_acquire(curthread) and never releases, deliberately handing
ownership to the teardown, so si_drv1 == curthread pid always holds on
a successful unload.  Every successful `kldload sound; kldunload
sound` without an intervening /dev/sndstat open+close therefore panics
deterministically (observed three times on the audit guest during
DF-4026/DF-4115 runs; minimal repro added by this finding).

Fix: initialize the sentinel to "not allocated" (-1).  sndstat_open()
already sets buflen = 0 right after a successful sbuf_new(), and
SNDSTAT_FLUSH() restores -1 after deleting, so the invariant becomes
consistent.  (FreeBSD solved the whole class by rewriting sndstat to
per-file cdevpriv sbufs with no global teardown; the one-liner below
is the minimal fix for DragonFly's architecture.)

--- a/sys/dev/sound/pcm/sndstat.c
+++ b/sys/dev/sound/pcm/sndstat.c
@@ -65,7 +65,7 @@
 static struct lock sndstat_lock;
 static struct sbuf sndstat_sbuf;
 static struct cdev *sndstat_dev = NULL;
-static int sndstat_buflen = 0;
+static int sndstat_buflen = -1;
 static int sndstat_maxunit = -1;
 static int sndstat_files = 0;