[PATCH] ALSA: Use del_timer_sync() before freeing timer

[Guenter Roeck]

On 10/26/22 20:12, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt at goodmis.org>
> 
> The current code for freeing the emux timer is extremely dangerous:
> 
>    CPU0				CPU1
>    ----				----
> snd_emux_timer_callback()
> 			    snd_emux_free()
> 			      spin_lock(&emu->voice_lock)
> 			      del_timer(&emu->tlist); <-- returns immediately
> 			      spin_unlock(&emu->voice_lock);
> 			      [..]
> 			      kfree(emu);
> 
>    spin_lock(&emu->voice_lock);
> 
>   [BOOM!]
> 
> Instead just use del_timer_sync() which will wait for the timer to finish
> before continuing. No need to check if the timer is active or not when
> doing so.
> 
> This doesn't fix the race of a possible re-arming of the timer, but at
> least it won't use the data that has just been freed.
> 
> Cc: stable at vger.kernel.org
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Steven Rostedt (Google) <rostedt at goodmis.org>

Reviewed-by: Guenter Roeck <linux at roeck-us.net>

> ---
>   sound/synth/emux/emux.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/sound/synth/emux/emux.c b/sound/synth/emux/emux.c
> index 5ed8e36d2e04..a2ee78809cfb 100644
> --- a/sound/synth/emux/emux.c
> +++ b/sound/synth/emux/emux.c
> @@ -131,10 +131,7 @@ int snd_emux_free(struct snd_emux *emu)
>   	if (! emu)
>   		return -EINVAL;
>   
> -	spin_lock_irqsave(&emu->voice_lock, flags);
> -	if (emu->timer_active)
> -		del_timer(&emu->tlist);
> -	spin_unlock_irqrestore(&emu->voice_lock, flags);
> +	del_timer_sync(&emu->tlist);
>   
>   	snd_emux_proc_free(emu);
>   	snd_emux_delete_virmidi(emu);