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

On Thu, Aug 13, 2026 at 07:09:44AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] The dynamically allocated `pair` is freed on stream shutdown without clearing the shared CPU DAI's `dma_data` pointer, leaving a dangling pointer that leads to a Use-After-Free on concurrent streams.
> - [High] The patch fails to resolve concurrent stream corruption because it assigns the per-pair `dma_params` to a shared CPU DAI, meaning concurrent streams overwrite each other's DMA parameters pointer.
> --
>
> --- Patch [3]: [PATCH v4 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct ---
> commit 6f9d240bd6e939329ded07360e8ef36835e62929
> Author: Shengjiu Wang <[email protected]>
>
> ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct
>
> The shared dma_params_tx/rx fields in struct fsl_asrc were a single
> instance written by every concurrent fsl_asrc_dma_hw_params() call.
> With multiple DAIs registered (one per pair/context), parallel stream
> open+hw_params races would corrupt addr and maxburst for all active
> streams.
>
> Fix by moving dma_params into struct fsl_asrc_pair so each pair owns
> its own copy. Initialise them in fsl_asrc_dma_startup() where
> the pair is allocated, which makes the per-DAI probe callbacks in
> fsl_asrc.c and fsl_easrc.c redundant; remove those as well.
>
> > diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
> > index 2f662bdf14d07..15f2dccc35840 100644
> > --- a/sound/soc/fsl/fsl_asrc_dma.c
> > +++ b/sound/soc/fsl/fsl_asrc_dma.c
> > @@ -392,6 +392,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: High]
> Does this actually resolve the concurrent stream corruption?
>
> If the driver registers only a single CPU DAI, won't multiple streams opening
> in the same direction all assign their per-pair `dma_params` to the same shared
> CPU DAI pointer?
>
> The last stream to open would overwrite the pointer. When concurrent streams
> then call fsl_asrc_dma_hw_params(), wouldn't they all fetch this same
> overwritten pointer and write their DMA configurations into a single shared
> struct, continuing to corrupt the data?
>
> [Severity: Critical]
> Can this also lead to a Use-After-Free when streams are closed?
>
> If Stream A opens and sets the shared CPU DAI pointer to its dynamically
> allocated `pair->dma_params`, and then Stream B opens concurrently and
> overwrites the shared pointer with its own `pair->dma_params`:
>
> If Stream B then closes and frees its `pair` struct via kfree() in
> fsl_asrc_dma_shutdown(), the pointer in the shared CPU DAI is left dangling.
>
> If Stream A then proceeds to fsl_asrc_dma_hw_params() and calls
> snd_soc_dai_get_dma_data(), won't it fetch the dangling pointer and write
> `addr` and `maxburst` into freed memory?


Please double check this.

Frank

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