| From 230323dac060123c340cf75997971145a42661ee Mon Sep 17 00:00:00 2001 |
| From: Takashi Iwai <tiwai@suse.de> |
| Date: Thu, 21 Jan 2016 17:19:31 +0100 |
| Subject: ALSA: timer: Handle disconnection more safely |
| |
| From: Takashi Iwai <tiwai@suse.de> |
| |
| commit 230323dac060123c340cf75997971145a42661ee upstream. |
| |
| Currently ALSA timer device doesn't take the disconnection into |
| account very well; it merely unlinks the timer device at disconnection |
| callback but does nothing else. Because of this, when an application |
| accessing the timer device is disconnected, it may release the |
| resource before actually closed. In most cases, it results in a |
| warning message indicating a leftover timer instance like: |
| ALSA: timer xxxx is busy? |
| But basically this is an open race. |
| |
| This patch tries to address it. The strategy is like other ALSA |
| devices: namely, |
| - Manage card's refcount at each open/close |
| - Wake up the pending tasks at disconnection |
| - Check the shutdown flag appropriately at each possible call |
| |
| Note that this patch has one ugly hack to handle the wakeup of pending |
| tasks. It'd be cleaner to introduce a new disconnect op to |
| snd_timer_instance ops. But since it would lead to internal ABI |
| breakage and it eventually increase my own work when backporting to |
| stable kernels, I took a different path to implement locally in |
| timer.c. A cleanup patch will follow at next for 4.5 kernel. |
| |
| Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109431 |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| sound/core/timer.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ |
| 1 file changed, 48 insertions(+) |
| |
| --- a/sound/core/timer.c |
| +++ b/sound/core/timer.c |
| @@ -65,6 +65,7 @@ struct snd_timer_user { |
| int qtail; |
| int qused; |
| int queue_size; |
| + bool disconnected; |
| struct snd_timer_read *queue; |
| struct snd_timer_tread *tqueue; |
| spinlock_t qlock; |
| @@ -290,6 +291,9 @@ int snd_timer_open(struct snd_timer_inst |
| mutex_unlock(®ister_mutex); |
| return -ENOMEM; |
| } |
| + /* take a card refcount for safe disconnection */ |
| + if (timer->card) |
| + get_device(&timer->card->card_dev); |
| timeri->slave_class = tid->dev_sclass; |
| timeri->slave_id = slave_id; |
| if (list_empty(&timer->open_list_head) && timer->hw.open) |
| @@ -360,6 +364,9 @@ int snd_timer_close(struct snd_timer_ins |
| } |
| spin_unlock(&timer->lock); |
| spin_unlock_irq(&slave_active_lock); |
| + /* release a card refcount for safe disconnection */ |
| + if (timer->card) |
| + put_device(&timer->card->card_dev); |
| mutex_unlock(®ister_mutex); |
| } |
| out: |
| @@ -475,6 +482,8 @@ int snd_timer_start(struct snd_timer_ins |
| timer = timeri->timer; |
| if (timer == NULL) |
| return -EINVAL; |
| + if (timer->card && timer->card->shutdown) |
| + return -ENODEV; |
| spin_lock_irqsave(&timer->lock, flags); |
| timeri->ticks = timeri->cticks = ticks; |
| timeri->pticks = 0; |
| @@ -509,6 +518,10 @@ static int _snd_timer_stop(struct snd_ti |
| spin_lock_irqsave(&timer->lock, flags); |
| list_del_init(&timeri->ack_list); |
| list_del_init(&timeri->active_list); |
| + if (timer->card && timer->card->shutdown) { |
| + spin_unlock_irqrestore(&timer->lock, flags); |
| + return 0; |
| + } |
| if ((timeri->flags & SNDRV_TIMER_IFLG_RUNNING) && |
| !(--timer->running)) { |
| timer->hw.stop(timer); |
| @@ -571,6 +584,8 @@ int snd_timer_continue(struct snd_timer_ |
| timer = timeri->timer; |
| if (! timer) |
| return -EINVAL; |
| + if (timer->card && timer->card->shutdown) |
| + return -ENODEV; |
| spin_lock_irqsave(&timer->lock, flags); |
| if (!timeri->cticks) |
| timeri->cticks = 1; |
| @@ -634,6 +649,9 @@ static void snd_timer_tasklet(unsigned l |
| unsigned long resolution, ticks; |
| unsigned long flags; |
| |
| + if (timer->card && timer->card->shutdown) |
| + return; |
| + |
| spin_lock_irqsave(&timer->lock, flags); |
| /* now process all callbacks */ |
| while (!list_empty(&timer->sack_list_head)) { |
| @@ -674,6 +692,9 @@ void snd_timer_interrupt(struct snd_time |
| if (timer == NULL) |
| return; |
| |
| + if (timer->card && timer->card->shutdown) |
| + return; |
| + |
| spin_lock_irqsave(&timer->lock, flags); |
| |
| /* remember the current resolution */ |
| @@ -884,11 +905,28 @@ static int snd_timer_dev_register(struct |
| return 0; |
| } |
| |
| +/* just for reference in snd_timer_dev_disconnect() below */ |
| +static void snd_timer_user_ccallback(struct snd_timer_instance *timeri, |
| + int event, struct timespec *tstamp, |
| + unsigned long resolution); |
| + |
| static int snd_timer_dev_disconnect(struct snd_device *device) |
| { |
| struct snd_timer *timer = device->device_data; |
| + struct snd_timer_instance *ti; |
| + |
| mutex_lock(®ister_mutex); |
| list_del_init(&timer->device_list); |
| + /* wake up pending sleepers */ |
| + list_for_each_entry(ti, &timer->open_list_head, open_list) { |
| + /* FIXME: better to have a ti.disconnect() op */ |
| + if (ti->ccallback == snd_timer_user_ccallback) { |
| + struct snd_timer_user *tu = ti->callback_data; |
| + |
| + tu->disconnected = true; |
| + wake_up(&tu->qchange_sleep); |
| + } |
| + } |
| mutex_unlock(®ister_mutex); |
| return 0; |
| } |
| @@ -899,6 +937,8 @@ void snd_timer_notify(struct snd_timer * |
| unsigned long resolution = 0; |
| struct snd_timer_instance *ti, *ts; |
| |
| + if (timer->card && timer->card->shutdown) |
| + return; |
| if (! (timer->hw.flags & SNDRV_TIMER_HW_SLAVE)) |
| return; |
| if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_MSTART || |
| @@ -1057,6 +1097,8 @@ static void snd_timer_proc_read(struct s |
| |
| mutex_lock(®ister_mutex); |
| list_for_each_entry(timer, &snd_timer_list, device_list) { |
| + if (timer->card && timer->card->shutdown) |
| + continue; |
| switch (timer->tmr_class) { |
| case SNDRV_TIMER_CLASS_GLOBAL: |
| snd_iprintf(buffer, "G%i: ", timer->tmr_device); |
| @@ -1882,6 +1924,10 @@ static ssize_t snd_timer_user_read(struc |
| |
| remove_wait_queue(&tu->qchange_sleep, &wait); |
| |
| + if (tu->disconnected) { |
| + err = -ENODEV; |
| + break; |
| + } |
| if (signal_pending(current)) { |
| err = -ERESTARTSYS; |
| break; |
| @@ -1931,6 +1977,8 @@ static unsigned int snd_timer_user_poll( |
| mask = 0; |
| if (tu->qused) |
| mask |= POLLIN | POLLRDNORM; |
| + if (tu->disconnected) |
| + mask |= POLLERR; |
| |
| return mask; |
| } |