Re: [PATCH v6 04/11] ASoC: renesas: fsi: Move fsi_stream_is_working()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Time-of-check to time-of-use (TOCTOU) race condition between fsi_stream_is_working() and subsequent lockless pointer access in fsi_pio_get_area(), leading to a potential NULL pointer dereference.
--

commit 8fa524754ea1b8f1ac9ac2d6e00b052f212f3a41
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: renesas: fsi: Move fsi_stream_is_working()
    
Move fsi_stream_is_working() before fsi_count_fifo_err(). This prepares for
a subsequent patch that needs to check stream status when handling in-flight
IRQ handlers.

> diff --git a/sound/soc/renesas/fsi.c b/sound/soc/renesas/fsi.c
> index 800ac40f9680d..0bd0e0c8c5a30 100644
> --- a/sound/soc/renesas/fsi.c
> +++ b/sound/soc/renesas/fsi.c
> @@ -442,6 +442,16 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples)
>  	return samples / fsi->chan_num;
>  }
>  
> +static int fsi_stream_is_working(struct fsi_priv *fsi,
> +				 struct fsi_stream *io)
> +{
> +	struct fsi_master *master = fsi_get_master(fsi);
> +
> +	guard(spinlock_irqsave)(&master->lock);
> +
> +	return !!(io->substream && io->substream->runtime);
> +}

[Severity: High]
This is a pre-existing issue, but does fsi_stream_is_working() leave callers
vulnerable to a time-of-check to time-of-use (TOCTOU) race condition?

The lock is acquired and then immediately released, protecting only the
condition check but not the caller's subsequent pointer usage. For example,
in fsi_pio_transfer(), the stream status is checked securely but the lock is
dropped, followed immediately by a lockless pointer access:

	if (!fsi_stream_is_working(fsi, io))
		return -EINVAL;

	buf = fsi_pio_get_area(fsi, io);

Concurrently, an ALSA stream stop triggered from userspace could call
fsi_stream_quit() on another CPU, which acquires the lock and sets
io->substream to NULL:

	guard(spinlock_irqsave)(&master->lock);
	...
	io->substream = NULL;

If fsi_stream_quit() acquires the lock immediately after
fsi_stream_is_working() releases it, the pointer would be nullified just
before the IRQ handler continues execution.

The IRQ handler would then proceed to call fsi_pio_get_area(), which
locklessly dereferences io->substream->runtime:

static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io)
{
	struct snd_pcm_runtime *runtime = io->substream->runtime;

Can this result in a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609113836.45079-1-phucduc.bui@gmail.com?part=4