ALSA: timer: Fix mutex deadlock at releasing card

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>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
Takashi Iwai 2019-10-30 22:42:57 +01:00
parent 302d5a80d2
commit a393318673
1 changed files with 17 additions and 7 deletions

View File

@ -226,7 +226,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
@ -238,6 +239,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);
@ -261,7 +263,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;
@ -313,7 +315,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;
}
@ -323,12 +325,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;
}
@ -338,7 +343,8 @@ EXPORT_SYMBOL(snd_timer_open);
* 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 = timeri->timer;
struct snd_timer_instance *slave, *tmp;
@ -395,7 +401,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);
}
@ -407,14 +413,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;
}
EXPORT_SYMBOL(snd_timer_close);