git: cc72812c49ba - stable/14 - sound: Fix NULL dereference in dsp_clone() and mixer_clone()

[Christos Margiolis]

The branch stable/14 has been updated by christos:

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

commit cc72812c49babc4b3a9bf904c91851e4921785fc
Author:     Christos Margiolis <chris...@freebsd.org>
AuthorDate: 2024-04-28 19:40:14 +0000
Commit:     Christos Margiolis <chris...@freebsd.org>
CommitDate: 2024-04-29 10:43:30 +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
    
    (cherry picked from commit 074d6fbebc160222cde6b726adcc7350881d7824)
---
 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 f685d7e38f6d..20625641a601 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 cc8cf5b1ceea..b84dfeba7043 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -1376,7 +1376,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 52ead91853e9..7de099cfe5ca 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -1018,10 +1018,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);
 
@@ -1039,6 +1035,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);