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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Mapping a dynamically allocated per-substream structure to a globally shared CPU DAI pointer reintroduces concurrent stream data races and causes a Use-After-Free (UAF) vulnerability.
--

--- Patch [3]: [PATCH v2 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct ---
commit 00cf71a6ea96804fe20d59cf4d7658270bb9bebf
Author: Shengjiu Wang <[email protected]>

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

This patch moves the dma_params fields into struct fsl_asrc_pair so each
concurrent stream pair owns its own copy. It removes the per-DAI probe
callbacks and assigns the per-pair DMA parameters to the CPU DAI during
startup.

> 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 setting the shared CPU DAI dma_data to a per-substream allocated
structure reintroduce data races and create a use-after-free regression?

Since snd_soc_dai is a singleton per component, concurrent streams might
overwrite this single pointer. If we have two concurrent streams mapping to
ASRC, stream 2 could overwrite the shared CPU DAI playback_dma_data to
point to its own pair2.

If stream 2 then closes or fails in startup, pair2 is freed:

sound/soc/fsl/fsl_asrc_dma.c:fsl_asrc_dma_shutdown() {
    ...
    kfree(pair);
    ...
}

The shared CPU DAI pointer is never nullified, leaving it dangling. If
stream 1 subsequently calls hw_params(), it would fetch this dangling
pointer:

sound/soc/fsl/fsl_asrc_dma.c:fsl_asrc_dma_hw_params() {
    ...
    dma_params_fe = snd_soc_dai_get_dma_data(snd_soc_rtd_to_cpu(rtd, 0),
                                             substream);
    dma_params_fe->addr = asrc->paddr + asrc->get_fifo_addr(!dir, index);
    dma_params_fe->maxburst = dma_params_be->maxburst;
    ...
}

Could this lead to cross-stream corruption or a use-after-free vulnerability
by writing to and reading from the freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3