ACK: [precise, trusty, vivid] ALSA: pcm : Call kill_fasync() in stream lock

Colin Ian King colin.king at canonical.com
Thu Dec 15 11:14:50 UTC 2016


On 15/12/16 11:08, Luis Henriques wrote:
> From: Takashi Iwai <tiwai at suse.de>
> 
> Currently kill_fasync() is called outside the stream lock in
> snd_pcm_period_elapsed().  This is potentially racy, since the stream
> may get released even during the irq handler is running.  Although
> snd_pcm_release_substream() calls snd_pcm_drop(), this doesn't
> guarantee that the irq handler finishes, thus the kill_fasync() call
> outside the stream spin lock may be invoked after the substream is
> detached, as recently reported by KASAN.
> 
> As a quick workaround, move kill_fasync() call inside the stream
> lock.  The fasync is rarely used interface, so this shouldn't have a
> big impact from the performance POV.
> 
> Ideally, we should implement some sync mechanism for the proper finish
> of stream and irq handler.  But this oneliner should suffice for most
> cases, so far.
> 
> Reported-by: Baozeng Ding <sploving1 at gmail.com>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> CVE-2016-9794
> (backported from commit 3aa02cb664c5fb1042958c8d1aa8c35055a2ebc4)
> [ luis: used Takashi's backport for upstream stable 3.12 ]
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  sound/core/pcm_lib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 7f00d34f0abd..dfbca788fbbc 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -1766,10 +1766,10 @@ void snd_pcm_period_elapsed(struct snd_pcm_substream *substream)
>  	if (substream->timer_running)
>  		snd_timer_interrupt(substream->timer, 1);
>   _end:
> +	kill_fasync(&runtime->fasync, SIGIO, POLL_IN);
>  	snd_pcm_stream_unlock_irqrestore(substream, flags);
>  	if (runtime->transfer_ack_end)
>  		runtime->transfer_ack_end(substream);
> -	kill_fasync(&runtime->fasync, SIGIO, POLL_IN);
>  }
>  
>  EXPORT_SYMBOL(snd_pcm_period_elapsed);
> 
Looks sane to me.

Acked-by: Colin Ian King <colin.king at canonical.com>




More information about the kernel-team mailing list