ALSA: timer: Fix mutex deadlock at releasing card

commit a39331867335d4a94b6165e306265c9e24aca073 upstream.

When a card is disconnected while in use, the system waits until all
opened files are closed then releases the card.  This is done via
put_device() of the card device in each device release code.

The recently reported mutex deadlock bug happens in this code path;
snd_timer_close() for the timer device deals with the global
register_mutex and it calls put_device() there.  When this timer
device is the last one, the card gets freed and it eventually calls
snd_timer_free(), which has again the protection with the global
register_mutex -- boom.

Basically put_device() call itself is race-free, so a relative simple
workaround is to move this put_device() call out of the mutex.  For
achieving that, in this patch, snd_timer_close_locked() got a new
argument to store the card device pointer in return, and each caller
invokes put_device() with the returned object after the mutex unlock.

Reported-and-tested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 698aafb4fc3cfb0564a0f06f58ce923e03fc094f..3be976ce583257b079d60c661ce902fc5db58962 100644 (file)
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -241,7 +241,8 @@ static int snd_timer_check_master(struct snd_timer_instance *master)
        return 0;
 }
 
-static int snd_timer_close_locked(struct snd_timer_instance *timeri);
+static int snd_timer_close_locked(struct snd_timer_instance *timeri,
+                                 struct device **card_devp_to_put);
 
 /*
  * open a timer instance
@@ -253,6 +254,7 @@ int snd_timer_open(struct snd_timer_instance **ti,
 {
        struct snd_timer *timer;
        struct snd_timer_instance *timeri = NULL;
+       struct device *card_dev_to_put = NULL;
        int err;
 
        mutex_lock(&register_mutex);
@@ -276,7 +278,7 @@ int snd_timer_open(struct snd_timer_instance **ti,
                list_add_tail(&timeri->open_list, &snd_timer_slave_list);
                err = snd_timer_check_slave(timeri);
                if (err < 0) {
-                       snd_timer_close_locked(timeri);
+                       snd_timer_close_locked(timeri, &card_dev_to_put);
                        timeri = NULL;
                }
                goto unlock;
@@ -328,7 +330,7 @@ int snd_timer_open(struct snd_timer_instance **ti,
                        timeri = NULL;
 
                        if (timer->card)
-                               put_device(&timer->card->card_dev);
+                               card_dev_to_put = &timer->card->card_dev;
                        module_put(timer->module);
                        goto unlock;
                }
@@ -338,12 +340,15 @@ int snd_timer_open(struct snd_timer_instance **ti,
        timer->num_instances++;
        err = snd_timer_check_master(timeri);
        if (err < 0) {
-               snd_timer_close_locked(timeri);
+               snd_timer_close_locked(timeri, &card_dev_to_put);
                timeri = NULL;
        }
 
  unlock:
        mutex_unlock(&register_mutex);
+       /* put_device() is called after unlock for avoiding deadlock */
+       if (card_dev_to_put)
+               put_device(card_dev_to_put);
        *ti = timeri;
        return err;
 }
@@ -352,7 +357,8 @@ int snd_timer_open(struct snd_timer_instance **ti,
  * close a timer instance
  * call this with register_mutex down.
  */
-static int snd_timer_close_locked(struct snd_timer_instance *timeri)
+static int snd_timer_close_locked(struct snd_timer_instance *timeri,
+                                 struct device **card_devp_to_put)
 {
        struct snd_timer *timer = NULL;
        struct snd_timer_instance *slave, *tmp;
@@ -404,7 +410,7 @@ static int snd_timer_close_locked(struct snd_timer_instance *timeri)
                        timer->hw.close(timer);
                /* release a card refcount for safe disconnection */
                if (timer->card)
-                       put_device(&timer->card->card_dev);
+                       *card_devp_to_put = &timer->card->card_dev;
                module_put(timer->module);
        }
 
@@ -416,14 +422,18 @@ static int snd_timer_close_locked(struct snd_timer_instance *timeri)
  */
 int snd_timer_close(struct snd_timer_instance *timeri)
 {
+       struct device *card_dev_to_put = NULL;
        int err;
 
        if (snd_BUG_ON(!timeri))
                return -ENXIO;
 
        mutex_lock(&register_mutex);
-       err = snd_timer_close_locked(timeri);
+       err = snd_timer_close_locked(timeri, &card_dev_to_put);
        mutex_unlock(&register_mutex);
+       /* put_device() is called after unlock for avoiding deadlock */
+       if (card_dev_to_put)
+               put_device(card_dev_to_put);
        return err;
 }