From 3ae180972564846e6d794e3615e1ab0a1e6c4ef9 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 17 May 2018 22:34:39 +0100 Subject: ALSA: timer: Fix pause event notification Commit f65e0d299807 ("ALSA: timer: Call notifier in the same spinlock") combined the start/continue and stop/pause functions, and in doing so changed the event code for the pause case to SNDRV_TIMER_EVENT_CONTINUE. Change it back to SNDRV_TIMER_EVENT_PAUSE. Fixes: f65e0d299807 ("ALSA: timer: Call notifier in the same spinlock") Signed-off-by: Ben Hutchings Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai --- sound/core/timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/core') diff --git a/sound/core/timer.c b/sound/core/timer.c index dc87728c5b74..0ddcae495838 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -592,7 +592,7 @@ static int snd_timer_stop1(struct snd_timer_instance *timeri, bool stop) else timeri->flags |= SNDRV_TIMER_IFLG_PAUSED; snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP : - SNDRV_TIMER_EVENT_CONTINUE); + SNDRV_TIMER_EVENT_PAUSE); unlock: spin_unlock_irqrestore(&timer->lock, flags); return result; @@ -614,7 +614,7 @@ static int snd_timer_stop_slave(struct snd_timer_instance *timeri, bool stop) list_del_init(&timeri->ack_list); list_del_init(&timeri->active_list); snd_timer_notify1(timeri, stop ? SNDRV_TIMER_EVENT_STOP : - SNDRV_TIMER_EVENT_CONTINUE); + SNDRV_TIMER_EVENT_PAUSE); spin_unlock(&timeri->timer->lock); } spin_unlock_irqrestore(&slave_active_lock, flags); -- cgit v1.2.3 From fdcb5761c1580b03fb1ab8625eaa7db03fb8b7d0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 16 May 2018 23:45:33 +0200 Subject: ALSA: timer: Simplify timer hw resolution calls There multiple open-codes to get the hardware timer resolution. Make a local helper function snd_timer_hw_resolution() and call it from all relevant places. There is no functional change by this, just a preliminary work for the following timer resolution hardening patch. Signed-off-by: Takashi Iwai --- sound/core/timer.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) (limited to 'sound/core') diff --git a/sound/core/timer.c b/sound/core/timer.c index 0ddcae495838..22c72857f379 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -427,6 +427,14 @@ int snd_timer_close(struct snd_timer_instance *timeri) } EXPORT_SYMBOL(snd_timer_close); +static unsigned long snd_timer_hw_resolution(struct snd_timer *timer) +{ + if (timer->hw.c_resolution) + return timer->hw.c_resolution(timer); + else + return timer->hw.resolution; +} + unsigned long snd_timer_resolution(struct snd_timer_instance *timeri) { struct snd_timer * timer; @@ -434,11 +442,8 @@ unsigned long snd_timer_resolution(struct snd_timer_instance *timeri) if (timeri == NULL) return 0; timer = timeri->timer; - if (timer) { - if (timer->hw.c_resolution) - return timer->hw.c_resolution(timer); - return timer->hw.resolution; - } + if (timer) + return snd_timer_hw_resolution(timer); return 0; } EXPORT_SYMBOL(snd_timer_resolution); @@ -771,10 +776,7 @@ void snd_timer_interrupt(struct snd_timer * timer, unsigned long ticks_left) spin_lock_irqsave(&timer->lock, flags); /* remember the current resolution */ - if (timer->hw.c_resolution) - resolution = timer->hw.c_resolution(timer); - else - resolution = timer->hw.resolution; + resolution = snd_timer_hw_resolution(timer); /* loop for all active instances * Here we cannot use list_for_each_entry because the active_list of a @@ -1014,12 +1016,8 @@ void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstam spin_lock_irqsave(&timer->lock, flags); if (event == SNDRV_TIMER_EVENT_MSTART || event == SNDRV_TIMER_EVENT_MCONTINUE || - event == SNDRV_TIMER_EVENT_MRESUME) { - if (timer->hw.c_resolution) - resolution = timer->hw.c_resolution(timer); - else - resolution = timer->hw.resolution; - } + event == SNDRV_TIMER_EVENT_MRESUME) + resolution = snd_timer_hw_resolution(timer); list_for_each_entry(ti, &timer->active_list_head, active_list) { if (ti->ccallback) ti->ccallback(ti, event, tstamp, resolution); @@ -1656,10 +1654,7 @@ static int snd_timer_user_gstatus(struct file *file, mutex_lock(®ister_mutex); t = snd_timer_find(&tid); if (t != NULL) { - if (t->hw.c_resolution) - gstatus.resolution = t->hw.c_resolution(t); - else - gstatus.resolution = t->hw.resolution; + gstatus.resolution = snd_timer_hw_resolution(t); if (t->hw.precise_resolution) { t->hw.precise_resolution(t, &gstatus.resolution_num, &gstatus.resolution_den); -- cgit v1.2.3 From 21244e3d6a9d36f32a2aa40f8948324c7b5f35b0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 17 May 2018 10:43:16 +0200 Subject: ALSA: seq: Avoid open-code for getting timer resolution Instead of open-coding for getting the timer resolution, use the standard snd_timer_resolution() helper. The original code falls back to the callback function when the resolution is zero, but it must be always so when the callback function is defined. So this should be no functional change. Signed-off-by: Takashi Iwai --- sound/core/seq/seq_timer.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'sound/core') diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c index 23167578231f..f587d0e27476 100644 --- a/sound/core/seq/seq_timer.c +++ b/sound/core/seq/seq_timer.c @@ -371,9 +371,7 @@ static int initialize_timer(struct snd_seq_timer *tmr) tmr->ticks = 1; if (!(t->hw.flags & SNDRV_TIMER_HW_SLAVE)) { - unsigned long r = t->hw.resolution; - if (! r && t->hw.c_resolution) - r = t->hw.c_resolution(t); + unsigned long r = snd_timer_resolution(tmr->timeri); if (r) { tmr->ticks = (unsigned int)(1000000000uL / (r * freq)); if (! tmr->ticks) -- cgit v1.2.3 From 9d4d207d1346329c5295420254f1dbef1a6ab6ba Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 16 May 2018 23:52:42 +0200 Subject: ALSA: timer: Assure timer resolution access always locked There are still many places calling the timer's hw.c_resolution callback without lock, and this may lead to some races, as we faced in the commit a820ccbe21e8 ("ALSA: pcm: Fix UAF at PCM release via PCM timer access"). This patch changes snd_timer_resolution() to take the timer->lock for avoiding the races. A place calling this function already inside the lock (from the notifier) is replaced with the snd_timer_hw_resolution() accordingly, as well as wrapping with the lock around another place calling snd_timer_hw_resolution(), too. Reported-by: Ben Hutchings Signed-off-by: Takashi Iwai --- sound/core/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'sound/core') diff --git a/sound/core/timer.c b/sound/core/timer.c index 22c72857f379..665089c45560 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -438,19 +438,24 @@ static unsigned long snd_timer_hw_resolution(struct snd_timer *timer) unsigned long snd_timer_resolution(struct snd_timer_instance *timeri) { struct snd_timer * timer; + unsigned long ret = 0; + unsigned long flags; if (timeri == NULL) return 0; timer = timeri->timer; - if (timer) - return snd_timer_hw_resolution(timer); - return 0; + if (timer) { + spin_lock_irqsave(&timer->lock, flags); + ret = snd_timer_hw_resolution(timer); + spin_unlock_irqrestore(&timer->lock, flags); + } + return ret; } EXPORT_SYMBOL(snd_timer_resolution); static void snd_timer_notify1(struct snd_timer_instance *ti, int event) { - struct snd_timer *timer; + struct snd_timer *timer = ti->timer; unsigned long resolution = 0; struct snd_timer_instance *ts; struct timespec tstamp; @@ -462,14 +467,14 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event) if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START || event > SNDRV_TIMER_EVENT_PAUSE)) return; - if (event == SNDRV_TIMER_EVENT_START || - event == SNDRV_TIMER_EVENT_CONTINUE) - resolution = snd_timer_resolution(ti); + if (timer && + (event == SNDRV_TIMER_EVENT_START || + event == SNDRV_TIMER_EVENT_CONTINUE)) + resolution = snd_timer_hw_resolution(timer); if (ti->ccallback) ti->ccallback(ti, event, &tstamp, resolution); if (ti->flags & SNDRV_TIMER_IFLG_SLAVE) return; - timer = ti->timer; if (timer == NULL) return; if (timer->hw.flags & SNDRV_TIMER_HW_SLAVE) @@ -1654,6 +1659,7 @@ static int snd_timer_user_gstatus(struct file *file, mutex_lock(®ister_mutex); t = snd_timer_find(&tid); if (t != NULL) { + spin_lock_irq(&t->lock); gstatus.resolution = snd_timer_hw_resolution(t); if (t->hw.precise_resolution) { t->hw.precise_resolution(t, &gstatus.resolution_num, @@ -1662,6 +1668,7 @@ static int snd_timer_user_gstatus(struct file *file, gstatus.resolution_num = gstatus.resolution; gstatus.resolution_den = 1000000000uL; } + spin_unlock_irq(&t->lock); } else { err = -ENODEV; } -- cgit v1.2.3