ALSA: aloop - Close races at restarting the stream
There are small races opened in the check of running bit and the timer
lock. Instead of adding yet more flag, just protect the whole racy
codes with the existing cable->lock. As a bonus, we can get rid of
timer_lock now.
Reported-and-tested-by: Omair Mohammed Abdullah <omair.m.abdullah@linux.intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index 0fe6d64..1904046 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -120,7 +120,6 @@
unsigned int last_drift;
unsigned long last_jiffies;
struct timer_list timer;
- spinlock_t timer_lock;
};
static struct platform_device *devices[SNDRV_CARDS];
@@ -166,12 +165,12 @@
return get_setup(dpcm)->rate_shift;
}
+/* call in cable->lock */
static void loopback_timer_start(struct loopback_pcm *dpcm)
{
unsigned long tick;
unsigned int rate_shift = get_rate_shift(dpcm);
- spin_lock(&dpcm->timer_lock);
if (rate_shift != dpcm->pcm_rate_shift) {
dpcm->pcm_rate_shift = rate_shift;
dpcm->period_size_frac = frac_pos(dpcm, dpcm->pcm_period_size);
@@ -184,15 +183,13 @@
tick = (tick + dpcm->pcm_bps - 1) / dpcm->pcm_bps;
dpcm->timer.expires = jiffies + tick;
add_timer(&dpcm->timer);
- spin_unlock(&dpcm->timer_lock);
}
+/* call in cable->lock */
static inline void loopback_timer_stop(struct loopback_pcm *dpcm)
{
- spin_lock(&dpcm->timer_lock);
del_timer(&dpcm->timer);
dpcm->timer.expires = 0;
- spin_unlock(&dpcm->timer_lock);
}
#define CABLE_VALID_PLAYBACK (1 << SNDRV_PCM_STREAM_PLAYBACK)
@@ -274,8 +271,8 @@
spin_lock(&cable->lock);
cable->running |= stream;
cable->pause &= ~stream;
- spin_unlock(&cable->lock);
loopback_timer_start(dpcm);
+ spin_unlock(&cable->lock);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
loopback_active_notify(dpcm);
break;
@@ -283,23 +280,23 @@
spin_lock(&cable->lock);
cable->running &= ~stream;
cable->pause &= ~stream;
- spin_unlock(&cable->lock);
loopback_timer_stop(dpcm);
+ spin_unlock(&cable->lock);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
loopback_active_notify(dpcm);
break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
spin_lock(&cable->lock);
cable->pause |= stream;
- spin_unlock(&cable->lock);
loopback_timer_stop(dpcm);
+ spin_unlock(&cable->lock);
break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
spin_lock(&cable->lock);
dpcm->last_jiffies = jiffies;
cable->pause &= ~stream;
- spin_unlock(&cable->lock);
loopback_timer_start(dpcm);
+ spin_unlock(&cable->lock);
break;
default:
return -EINVAL;
@@ -477,6 +474,7 @@
dpcm->buf_pos %= dpcm->pcm_buffer_size;
}
+/* call in cable->lock */
static unsigned int loopback_pos_update(struct loopback_cable *cable)
{
struct loopback_pcm *dpcm_play =
@@ -485,9 +483,7 @@
cable->streams[SNDRV_PCM_STREAM_CAPTURE];
unsigned long delta_play = 0, delta_capt = 0;
unsigned int running, count1, count2;
- unsigned long flags;
- spin_lock_irqsave(&cable->lock, flags);
running = cable->running ^ cable->pause;
if (running & (1 << SNDRV_PCM_STREAM_PLAYBACK)) {
delta_play = jiffies - dpcm_play->last_jiffies;
@@ -529,32 +525,39 @@
bytepos_finish(dpcm_play, count1);
bytepos_finish(dpcm_capt, count1);
unlock:
- spin_unlock_irqrestore(&cable->lock, flags);
return running;
}
static void loopback_timer_function(unsigned long data)
{
struct loopback_pcm *dpcm = (struct loopback_pcm *)data;
- unsigned int running;
+ unsigned long flags;
- running = loopback_pos_update(dpcm->cable);
- if (running & (1 << dpcm->substream->stream)) {
+ spin_lock_irqsave(&dpcm->cable->lock, flags);
+ if (loopback_pos_update(dpcm->cable) & (1 << dpcm->substream->stream)) {
loopback_timer_start(dpcm);
if (dpcm->period_update_pending) {
dpcm->period_update_pending = 0;
+ spin_unlock_irqrestore(&dpcm->cable->lock, flags);
+ /* need to unlock before calling below */
snd_pcm_period_elapsed(dpcm->substream);
+ return;
}
}
+ spin_unlock_irqrestore(&dpcm->cable->lock, flags);
}
static snd_pcm_uframes_t loopback_pointer(struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct loopback_pcm *dpcm = runtime->private_data;
+ snd_pcm_uframes_t pos;
+ spin_lock(&dpcm->cable->lock);
loopback_pos_update(dpcm->cable);
- return bytes_to_frames(runtime, dpcm->buf_pos);
+ pos = dpcm->buf_pos;
+ spin_unlock(&dpcm->cable->lock);
+ return bytes_to_frames(runtime, pos);
}
static struct snd_pcm_hardware loopback_pcm_hardware =
@@ -672,7 +675,6 @@
dpcm->substream = substream;
setup_timer(&dpcm->timer, loopback_timer_function,
(unsigned long)dpcm);
- spin_lock_init(&dpcm->timer_lock);
cable = loopback->cables[substream->number][dev];
if (!cable) {