Re: [PATCH v3 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Use-After-Free and Write-After-Free due to shared CPU DAI `dma_data` being overwritten by concurrent substreams.

Pre-existing issues:
- [High] Failed `hw_params` requests cause `hw_free` to unconditionally release a stale pair index, potentially disabling another active stream's hardware context.
--

--- Patch [3]: [PATCH v3 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct ---
commit 4551053ba617a888e5b04cb058570e71617e9706
Author: Shengjiu Wang <shengjiu.wang@nxp.com>

ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct

This patch moves the DMA parameters for transmit and receive paths from
the shared fsl_asrc structure into the per-pair structure. This aims to
prevent concurrent stream initializations from overwriting each other's
addresses and maxburst values. Redundant per-DAI probe callbacks are
removed as well.

> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index df4817ae91b18..2755ee57c6a17 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
[ ... ]
> @@ -781,18 +781,7 @@ static int fsl_asrc_dai_trigger(struct snd_pcm_substream *substream, int cmd,
>  	return 0;
>  }
>  
> -static int fsl_asrc_dai_probe(struct snd_soc_dai *dai)
> -{
> -	struct fsl_asrc *asrc = snd_soc_dai_get_drvdata(dai);
> -
> -	snd_soc_dai_init_dma_data(dai, &asrc->dma_params_tx,
> -				  &asrc->dma_params_rx);
> -
> -	return 0;
> -}
> -
>  static const struct snd_soc_dai_ops fsl_asrc_dai_ops = {
> -	.probe		= fsl_asrc_dai_probe,
>  	.startup	= fsl_asrc_dai_startup,
>  	.hw_params	= fsl_asrc_dai_hw_params,
>  	.hw_free	= fsl_asrc_dai_hw_free,

[Severity: High]
This is a pre-existing issue, but could a failure in fsl_asrc_dai_hw_params()
cause problems here in fsl_asrc_dai_hw_free()?

If fsl_asrc_request_pair() fails during hw_params (e.g. because no channels
are available), the ALSA core automatically calls fsl_asrc_dai_hw_free() to
clean up:

	if (pair)
		fsl_asrc_release_pair(pair);

Since pair->index still retains the stale index of the dummy pair acquired
and released earlier in fsl_asrc_dma_startup(), this unconditionally releases
that index:

	regmap_update_bits(asrc->regmap, REG_ASRCTR,
			   ASRCTR_ASRCEi_MASK(index), 0);

If another concurrent stream has since acquired that pair index, could this
abruptly stop its hardware conversion and corrupt its state?

> diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
> index 38f2b7c63133a..64d66420e7ada 100644
> --- a/sound/soc/fsl/fsl_asrc_dma.c
> +++ b/sound/soc/fsl/fsl_asrc_dma.c
> @@ -390,6 +390,20 @@ static int fsl_asrc_dma_startup(struct snd_soc_component *component,
>  
>  	runtime->private_data = pair;
>  
> +	/*
> +	 * Point the cpu DAI dma_data at the per-pair params so that
> +	 * concurrent hw_params calls on different pairs each write to
> +	 * their own struct and do not race on addr/maxburst.  Use the
> +	 * per-direction setters so that a concurrent open of the other
> +	 * direction on the same DAI does not NULL out its pointer.
> +	 */
> +	if (tx)
> +		snd_soc_dai_dma_data_set_playback(snd_soc_rtd_to_cpu(rtd, 0),
> +						  &pair->dma_params);
> +	else
> +		snd_soc_dai_dma_data_set_capture(snd_soc_rtd_to_cpu(rtd, 0),
> +						 &pair->dma_params);
> +

[Severity: Critical]
Does this code overwrite the shared CPU DAI's dma_data pointer with a
per-substream dynamically allocated pointer?

Since the CPU DAI is shared across multiple substreams, if this startup
sequence fails later (e.g. no channels available) and kfree(pair) is called
in the error path:

	if (release_pair)
		kfree(pair);

Could this leave the CPU DAI with a dangling pointer?

Similarly, when a stream is closed and fsl_asrc_dma_shutdown() calls
kfree(pair), does it leave any remaining active streams with a dangling
pointer in the shared DAI, leading to a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804100218.49587-1-shengjiu.wang@oss.nxp.com?part=3