ALSA: usb-audio: fix oops due to cleanup race when disconnecting

When a USB audio device is disconnected, snd_usb_audio_disconnect()
kills all audio URBs.  At the same time, the application, after being
notified of the disconnection, might close the device, in which case
ALSA calls the .hw_free callback, which should free the URBs too.

Commit de1b8b93a0ba "[ALSA] Fix hang-up at disconnection of usb-audio"
prevented snd_usb_hw_free() from freeing the URBs to avoid a hang that
resulted from this race, but this introduced another race because the
URB callbacks could now be executed after snd_usb_hw_free() has
returned, and try to access already freed data.

Fix the first race by introducing a mutex to serialize the disconnect
callback and all PCM callbacks that manage URBs (hw_free and hw_params).

Reported-and-tested-by: Pierre-Louis Bossart <pierre-louis.bossart@intel.com>
Cc: <stable@kernel.org>
[CL: also serialize hw_params callback]
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 800f7cb4f251cd77ae246c5dee2acaed9de587f4..c0f8270bc199e9e91a82cef451d1aa63ead9eb15 100644 (file)
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -323,6 +323,7 @@ static int snd_usb_audio_create(struct usb_device *dev, int idx,
                return -ENOMEM;
        }
 
+       mutex_init(&chip->shutdown_mutex);
        chip->index = idx;
        chip->dev = dev;
        chip->card = card;
@@ -531,6 +532,7 @@ static void snd_usb_audio_disconnect(struct usb_device *dev, void *ptr)
        chip = ptr;
        card = chip->card;
        mutex_lock(&register_mutex);
+       mutex_lock(&chip->shutdown_mutex);
        chip->shutdown = 1;
        chip->num_interfaces--;
        if (chip->num_interfaces <= 0) {
@@ -548,9 +550,11 @@ static void snd_usb_audio_disconnect(struct usb_device *dev, void *ptr)
                        snd_usb_mixer_disconnect(p);
                }
                usb_chip[chip->index] = NULL;
+               mutex_unlock(&chip->shutdown_mutex);
                mutex_unlock(&register_mutex);
                snd_card_free_when_closed(card);
        } else {
+               mutex_unlock(&chip->shutdown_mutex);
                mutex_unlock(&register_mutex);
        }
 }

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 4132522ac90f8c88da831b68f9cbdf777131de8e..e3f680526cb5ce840787d7feba00cdf7e54ff2f5 100644 (file)
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -361,6 +361,7 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
        }
 
        if (changed) {
+               mutex_lock(&subs->stream->chip->shutdown_mutex);
                /* format changed */
                snd_usb_release_substream_urbs(subs, 0);
                /* influenced: period_bytes, channels, rate, format, */
@@ -368,6 +369,7 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
                                                  params_rate(hw_params),
                                                  snd_pcm_format_physical_width(params_format(hw_params)) *
                                                        params_channels(hw_params));
+               mutex_unlock(&subs->stream->chip->shutdown_mutex);
        }
 
        return ret;
@@ -385,8 +387,9 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
        subs->cur_audiofmt = NULL;
        subs->cur_rate = 0;
        subs->period_bytes = 0;
-       if (!subs->stream->chip->shutdown)
-               snd_usb_release_substream_urbs(subs, 0);
+       mutex_lock(&subs->stream->chip->shutdown_mutex);
+       snd_usb_release_substream_urbs(subs, 0);
+       mutex_unlock(&subs->stream->chip->shutdown_mutex);
        return snd_pcm_lib_free_vmalloc_buffer(substream);
 }
 

diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index db3eb21627eeee5f7c9c062286e5669ed93aff1a..6e66fffe87f57096bbcb9abf1cbe453d075400ab 100644 (file)
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -36,6 +36,7 @@ struct snd_usb_audio {
        struct snd_card *card;
        u32 usb_id;
        int shutdown;
+       struct mutex shutdown_mutex;
        unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */
        int num_interfaces;
        int num_suspended_intf;