commit 230323dac060123c340cf75997971145a42661ee
author Takashi Iwai <tiwai@suse.de> Thu Jan 21 17:19:31 2016 +0100
committer Takashi Iwai <tiwai@suse.de> Thu Jan 21 17:41:50 2016 +0100
tree aaa5dc1f15cbe38b1d051460d5a485c2d1c63c15
parent 991f86d7ae4e1f8c15806e62f97af519e3cdd860

ALSA: timer: Handle disconnection more safely

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
Cc: <stable@vger.kernel.org> # v3.15+
Signed-off-by: Takashi Iwai <tiwai@suse.de>

sound/core/timer.c

diff --git a/sound/core/timer.c b/sound/core/timer.c
index cb25ade..681fb05 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -65,6 +65,7 @@
 	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 @@
 		mutex_unlock(&register_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)
@@ -359,6 +363,9 @@
 		}
 		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(&register_mutex);
 	}
  out:
@@ -474,6 +481,8 @@
 	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;
@@ -505,6 +514,10 @@
 	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);
@@ -565,6 +578,8 @@
 	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;
@@ -628,6 +643,9 @@
 	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)) {
@@ -668,6 +686,9 @@
 	if (timer == NULL)
 		return;
 
+	if (timer->card && timer->card->shutdown)
+		return;
+
 	spin_lock_irqsave(&timer->lock, flags);
 
 	/* remember the current resolution */
@@ -878,11 +899,28 @@
 	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(&register_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(&register_mutex);
 	return 0;
 }
@@ -893,6 +931,8 @@
 	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 ||
@@ -1051,6 +1091,8 @@
 
 	mutex_lock(&register_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);
@@ -1876,6 +1918,10 @@
 
 			remove_wait_queue(&tu->qchange_sleep, &wait);
 
+			if (tu->disconnected) {
+				err = -ENODEV;
+				break;
+			}
 			if (signal_pending(current)) {
 				err = -ERESTARTSYS;
 				break;
@@ -1925,6 +1971,8 @@
 	mask = 0;
 	if (tu->qused)
 		mask |= POLLIN | POLLRDNORM;
+	if (tu->disconnected)
+		mask |= POLLERR;
 
 	return mask;
 }