git: 074d6fbebc16 - main - sound: Fix NULL dereference in dsp_clone() and mixer_clone()

From: Christos Margiolis <christos_at_FreeBSD.org>
Date: Sun, 28 Apr 2024 19:48:59 UTC
The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=074d6fbebc160222cde6b726adcc7350881d7824

commit 074d6fbebc160222cde6b726adcc7350881d7824
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-04-28 19:40:14 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-04-28 19:48:24 +0000

    sound: Fix NULL dereference in dsp_clone() and mixer_clone()
    
    If we only have a single soundcard attached and we detach it right
    before entering [dsp|mixer]_clone(), there is a chance pcm_unregister()
    will have returned already, meaning it will have set snd_unit to -1, and
    thus devclass_get_softc() will return NULL here.
    
    While here, 1) move the calls to dsp_destroy_dev() and mixer_uninit()
    below the point where we unset SD_F_REGISTERED, and 2) follow what
    mixer_clone() does and make sure we don't use a NULL d->dsp_dev in
    dsp_clone().
    
    Reported by:    KASAN
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 day
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D44924
---
 sys/dev/sound/pcm/dsp.c   | 14 ++++++++++----
 sys/dev/sound/pcm/mixer.c |  3 ++-
 sys/dev/sound/pcm/sound.c |  7 +++----
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 1c020ba22611..c5bb27530d2b 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -2086,10 +2086,16 @@ dsp_clone(void *arg, struct ucred *cred, char *name, int namelen,
 	return;
 found:
 	d = devclass_get_softc(pcm_devclass, snd_unit);
-	if (!PCM_REGISTERED(d))
-		return;
-	*dev = d->dsp_dev;
-	dev_ref(*dev);
+	/*
+	 * If we only have a single soundcard attached and we detach it right
+	 * before entering dsp_clone(), there is a chance pcm_unregister() will
+	 * have returned already, meaning it will have set snd_unit to -1, and
+	 * thus devclass_get_softc() will return NULL here.
+	 */
+	if (d != NULL && PCM_REGISTERED(d) && d->dsp_dev != NULL) {
+		*dev = d->dsp_dev;
+		dev_ref(*dev);
+	}
 }
 
 static void
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index 0178e1ae8c3e..33a7eb26606d 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -1378,7 +1378,8 @@ mixer_clone(void *arg,
 		return;
 	if (strcmp(name, "mixer") == 0) {
 		d = devclass_get_softc(pcm_devclass, snd_unit);
-		if (PCM_REGISTERED(d) && d->mixer_dev != NULL) {
+		/* See related comment in dsp_clone(). */
+		if (d != NULL && PCM_REGISTERED(d) && d->mixer_dev != NULL) {
 			*dev = d->mixer_dev;
 			dev_ref(*dev);
 		}
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index e88ccb1f25a6..faf4bd1c7dce 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -1013,10 +1013,6 @@ pcm_unregister(device_t dev)
 		CHN_UNLOCK(ch);
 	}
 
-	dsp_destroy_dev(dev);
-
-	(void)mixer_uninit(dev);
-
 	/* remove /dev/sndstat entry first */
 	sndstat_unregister(dev);
 
@@ -1034,6 +1030,9 @@ pcm_unregister(device_t dev)
 		d->rec_sysctl_tree = NULL;
 	}
 
+	dsp_destroy_dev(dev);
+	(void)mixer_uninit(dev);
+
 	while (!CHN_EMPTY(d, channels.pcm))
 		pcm_killchan(dev);