On Mon, 20 Mar 2017 18:09:15 +0100,
Arnaud Pouliquen wrote:
>
> With RTlinux a race condition has been found that leads to NULL ptr crash:
> - On CPU 0: uni_player_irq_handler is called to treat XRUN
> "(player->state == UNIPERIF_STATE_STOPPED)" is FALSE so status is checked,
> dev_err(player->dev, "FIFO underflow error detected") is printed
> and then snd_pcm_stream_lock should be called to lock stream for stopping.
> - On CPU 1: application stop and close the stream.
> Issue is that the stop and shutdown functions are executed while
> "FIFO underflow error detected" is printed.
> So when CPU 0 call snd_pcm_stream_lock is player->substream is already null.
Hrm, how the first call of snd_pcm_stream_lock() with
player->substream to be guaranteed to be non-NULL?
Takashi
>
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxx>
> ---
> sound/soc/sti/uniperif_player.c | 9 +++------
> sound/soc/sti/uniperif_reader.c | 8 +++++---
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c
> index 60ae31a..6fa9eee 100644
> --- a/sound/soc/sti/uniperif_player.c
> +++ b/sound/soc/sti/uniperif_player.c
> @@ -65,8 +65,10 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id)
> unsigned int status;
> unsigned int tmp;
>
> + snd_pcm_stream_lock(player->substream);
> if (player->state == UNIPERIF_STATE_STOPPED) {
> /* Unexpected IRQ: do nothing */
> + snd_pcm_stream_unlock(player->substream);
> return IRQ_NONE;
> }
>
> @@ -88,9 +90,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id)
> SET_UNIPERIF_ITM_BCLR_FIFO_ERROR(player);
>
> /* Stop the player */
> - snd_pcm_stream_lock(player->substream);
> snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
> - snd_pcm_stream_unlock(player->substream);
> }
>
> ret = IRQ_HANDLED;
> @@ -104,9 +104,7 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id)
> SET_UNIPERIF_ITM_BCLR_DMA_ERROR(player);
>
> /* Stop the player */
> - snd_pcm_stream_lock(player->substream);
> snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
> - snd_pcm_stream_unlock(player->substream);
>
> ret = IRQ_HANDLED;
> }
> @@ -138,13 +136,12 @@ static irqreturn_t uni_player_irq_handler(int irq, void *dev_id)
> dev_err(player->dev, "Underflow recovery failed\n");
>
> /* Stop the player */
> - snd_pcm_stream_lock(player->substream);
> snd_pcm_stop(player->substream, SNDRV_PCM_STATE_XRUN);
> - snd_pcm_stream_unlock(player->substream);
>
> ret = IRQ_HANDLED;
> }
>
> + snd_pcm_stream_unlock(player->substream);
> return ret;
> }
>
> diff --git a/sound/soc/sti/uniperif_reader.c b/sound/soc/sti/uniperif_reader.c
> index 5992c6a..9273f59 100644
> --- a/sound/soc/sti/uniperif_reader.c
> +++ b/sound/soc/sti/uniperif_reader.c
> @@ -46,9 +46,11 @@ static irqreturn_t uni_reader_irq_handler(int irq, void *dev_id)
> struct uniperif *reader = dev_id;
> unsigned int status;
>
> + snd_pcm_stream_lock(reader->substream);
> if (reader->state == UNIPERIF_STATE_STOPPED) {
> /* Unexpected IRQ: do nothing */
> dev_warn(reader->dev, "unexpected IRQ\n");
> + snd_pcm_stream_unlock(reader->substream);
> return IRQ_HANDLED;
> }
>
> @@ -60,13 +62,13 @@ static irqreturn_t uni_reader_irq_handler(int irq, void *dev_id)
> if (unlikely(status & UNIPERIF_ITS_FIFO_ERROR_MASK(reader))) {
> dev_err(reader->dev, "FIFO error detected\n");
>
> - snd_pcm_stream_lock(reader->substream);
> snd_pcm_stop(reader->substream, SNDRV_PCM_STATE_XRUN);
> - snd_pcm_stream_unlock(reader->substream);
>
> - return IRQ_HANDLED;
> + ret = IRQ_HANDLED;
> }
>
> + snd_pcm_stream_unlock(reader->substream);
> +
> return ret;
> }
>
> --
> 1.9.1
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
