mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	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: Dmitry Vyukov <dvyukov@google.com> Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
		
							parent
							
								
									2cdc7b636d
								
							
						
					
					
						commit
						f784beb75c
					
				
					 1 changed files with 28 additions and 2 deletions
				
			
		| 
						 | 
					@ -451,6 +451,10 @@ static int snd_timer_start_slave(struct snd_timer_instance *timeri)
 | 
				
			||||||
	unsigned long flags;
 | 
						unsigned long flags;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	spin_lock_irqsave(&slave_active_lock, 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;
 | 
						timeri->flags |= SNDRV_TIMER_IFLG_RUNNING;
 | 
				
			||||||
	if (timeri->master && timeri->timer) {
 | 
						if (timeri->master && timeri->timer) {
 | 
				
			||||||
		spin_lock(&timeri->timer->lock);
 | 
							spin_lock(&timeri->timer->lock);
 | 
				
			||||||
| 
						 | 
					@ -475,7 +479,8 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
 | 
				
			||||||
		return -EINVAL;
 | 
							return -EINVAL;
 | 
				
			||||||
	if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
 | 
						if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
 | 
				
			||||||
		result = snd_timer_start_slave(timeri);
 | 
							result = snd_timer_start_slave(timeri);
 | 
				
			||||||
		snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
 | 
							if (result >= 0)
 | 
				
			||||||
 | 
								snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
 | 
				
			||||||
		return result;
 | 
							return result;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	timer = timeri->timer;
 | 
						timer = timeri->timer;
 | 
				
			||||||
| 
						 | 
					@ -484,11 +489,18 @@ int snd_timer_start(struct snd_timer_instance *timeri, unsigned int ticks)
 | 
				
			||||||
	if (timer->card && timer->card->shutdown)
 | 
						if (timer->card && timer->card->shutdown)
 | 
				
			||||||
		return -ENODEV;
 | 
							return -ENODEV;
 | 
				
			||||||
	spin_lock_irqsave(&timer->lock, flags);
 | 
						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->ticks = timeri->cticks = ticks;
 | 
				
			||||||
	timeri->pticks = 0;
 | 
						timeri->pticks = 0;
 | 
				
			||||||
	result = snd_timer_start1(timer, timeri, ticks);
 | 
						result = snd_timer_start1(timer, timeri, ticks);
 | 
				
			||||||
 | 
					 unlock:
 | 
				
			||||||
	spin_unlock_irqrestore(&timer->lock, flags);
 | 
						spin_unlock_irqrestore(&timer->lock, flags);
 | 
				
			||||||
	snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
 | 
						if (result >= 0)
 | 
				
			||||||
 | 
							snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_START);
 | 
				
			||||||
	return result;
 | 
						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) {
 | 
						if (timeri->flags & SNDRV_TIMER_IFLG_SLAVE) {
 | 
				
			||||||
		spin_lock_irqsave(&slave_active_lock, 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;
 | 
							timeri->flags &= ~SNDRV_TIMER_IFLG_RUNNING;
 | 
				
			||||||
		list_del_init(&timeri->ack_list);
 | 
							list_del_init(&timeri->ack_list);
 | 
				
			||||||
		list_del_init(&timeri->active_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)
 | 
						if (!timer)
 | 
				
			||||||
		return -EINVAL;
 | 
							return -EINVAL;
 | 
				
			||||||
	spin_lock_irqsave(&timer->lock, flags);
 | 
						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->ack_list);
 | 
				
			||||||
	list_del_init(&timeri->active_list);
 | 
						list_del_init(&timeri->active_list);
 | 
				
			||||||
	if (timer->card && timer->card->shutdown) {
 | 
						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)
 | 
						if (timer->card && timer->card->shutdown)
 | 
				
			||||||
		return -ENODEV;
 | 
							return -ENODEV;
 | 
				
			||||||
	spin_lock_irqsave(&timer->lock, flags);
 | 
						spin_lock_irqsave(&timer->lock, flags);
 | 
				
			||||||
 | 
						if (timeri->flags & SNDRV_TIMER_IFLG_RUNNING) {
 | 
				
			||||||
 | 
							result = -EBUSY;
 | 
				
			||||||
 | 
							goto unlock;
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
	if (!timeri->cticks)
 | 
						if (!timeri->cticks)
 | 
				
			||||||
		timeri->cticks = 1;
 | 
							timeri->cticks = 1;
 | 
				
			||||||
	timeri->pticks = 0;
 | 
						timeri->pticks = 0;
 | 
				
			||||||
	result = snd_timer_start1(timer, timeri, timer->sticks);
 | 
						result = snd_timer_start1(timer, timeri, timer->sticks);
 | 
				
			||||||
 | 
					 unlock:
 | 
				
			||||||
	spin_unlock_irqrestore(&timer->lock, flags);
 | 
						spin_unlock_irqrestore(&timer->lock, flags);
 | 
				
			||||||
	snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE);
 | 
						snd_timer_notify1(timeri, SNDRV_TIMER_EVENT_CONTINUE);
 | 
				
			||||||
	return result;
 | 
						return result;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue