Thread (16 messages) flat view 16 messages, 5 authors, 23d ago

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

From: sashiko-bot@kernel.org
Date: 2026-08-13 07:09:45
Also in: imx

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813070020.719810-1-shengjiu.wang@oss.nxp.com?part=3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help