Commit f784beb7 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: timer: Fix link corruption due to double start or stop 

Although ALSA timer code got hardening for races, it still causes
use-after-free error.  This is however rather a corrupted linked list,
not actually the concurrent accesses.  Namely, when timer start is
triggered twice, list_add_tail() is called twice, too.  This ends
up with the link corruption and triggers KASAN error.

The simplest fix would be replacing list_add_tail() with
list_move_tail(), but fundamentally it's the problem that we don't
check the double start/stop correctly.  So, the right fix here is to
add the proper checks to snd_timer_start() and snd_timer_stop() (and
their variants).

BugLink: http://lkml.kernel.org/r/CACT4Y+ZyPRoMQjmawbvmCEDrkBD2BQuH7R09=eOkf5ESK8kJAw@mail.gmail.com


Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 2cdc7b63

sound/core/timer.c

+28 −2
Original line number Diff line number Diff line
@@ -451,6 +451,10 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri) 
	unsigned long flags;



	spin_lock_irqsave(&slave_active_lock, flags);

	if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {

		spin_unlock_irqrestore(&slave_active_lock, flags);

		return -EBUSY;

	}

	timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;

	if (timeri->master && timeri->timer) {

		spin_lock(&timeri->timer->lock);
@@ -475,6 +479,7 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) 
		return -EINVAL;

	if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {

		result = snd_timer_start_slave(timeri);

		if (result >= 0)

			snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);

		return result;

	}
@@ -484,10 +489,17 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks) 
	if (timer->card && timer->card->shutdown)

		return -ENODEV;

	spin_lock_irqsave(&timer->lock, flags);

	if (timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |

			     SNDRV_TIMER_IFLG_START)) {

		result = -EBUSY;

		goto unlock;

	}

	timeri->ticks = timeri->cticks = ticks;

	timeri->pticks = 0;

	result = snd_timer_start1(timer, timeri, ticks);

 unlock:

	spin_unlock_irqrestore(&timer->lock, flags);

	if (result >= 0)

		snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);

	return result;

}
@@ -502,6 +514,10 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) 


	if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {

		spin_lock_irqsave(&slave_active_lock, flags);

		if (!(timeri->flags & SNDRV_TIMER_IFLG_RUNNING)) {

			spin_unlock_irqrestore(&slave_active_lock, flags);

			return -EBUSY;

		}

		timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;

		list_del_init(&timeri->ack_list);

		list_del_init(&timeri->active_list);
@@ -512,6 +528,11 @@ static int _snd_timer_stop(struct snd_timer_instance *timeri, int event) 
	if (!timer)

		return -EINVAL;

	spin_lock_irqsave(&timer->lock, flags);

	if (!(timeri->flags & (SNDRV_TIMER_IFLG_RUNNING |

			       SNDRV_TIMER_IFLG_START))) {

		spin_unlock_irqrestore(&timer->lock, flags);

		return -EBUSY;

	}

	list_del_init(&timeri->ack_list);

	list_del_init(&timeri->active_list);

	if (timer->card && timer->card->shutdown) {
@@ -581,10 +602,15 @@ int snd_timer_continue(struct snd_timer_instance *timeri) 
	if (timer->card && timer->card->shutdown)

		return -ENODEV;

	spin_lock_irqsave(&timer->lock, flags);

	if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {

		result = -EBUSY;

		goto unlock;

	}

	if (!timeri->cticks)

		timeri->cticks = 1;

	timeri->pticks = 0;

	result = snd_timer_start1(timer, timeri, timer->sticks);

 unlock:

	spin_unlock_irqrestore(&timer->lock, flags);

	snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE);

	return result;

CRA Git | Maintained and supported by SUSTech CRA and CCSE