From: Frank Li <Frank.li@oss.nxp.com>
To: sashiko-reviews@lists.linux.dev
Cc: shengjiu.wang@oss.nxp.com, devicetree@vger.kernel.org,
Frank.Li@kernel.org, imx@lists.linux.dev, conor+dt@kernel.org,
robh@kernel.org
Subject: Re: [PATCH v4 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct
Date: Thu, 13 Aug 2026 11:54:26 -0500 [thread overview]
Message-ID: <an32wu6oTkCAwoe1@SMW015318> (raw)
In-Reply-To: <20260813070944.C22CF1F000E9@smtp.kernel.org>
On Thu, Aug 13, 2026 at 07:09:44AM +0000, sashiko-bot@kernel.org 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 <shengjiu.wang@nxp.com>
>
> 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/20260813070020.719810-1-shengjiu.wang@oss.nxp.com?part=3
next prev parent reply other threads:[~2026-08-13 16:54 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 7:00 [PATCH v4 0/5] ASoC: fsl_asrc/fsl_easrc: expose per-pair/context DAIs and fix DMA race shengjiu.wang
2026-08-13 7:00 ` [PATCH v4 1/5] ASoC: dt-bindings: fsl,imx-asrc: update port binding to support multiple paths shengjiu.wang
2026-08-14 7:21 ` Krzysztof Kozlowski
2026-08-14 7:41 ` Shengjiu Wang
2026-08-13 7:00 ` [PATCH v4 2/5] ASoC: dt-bindings: fsl,easrc: add ports binding for multiple conversion paths shengjiu.wang
2026-08-14 7:22 ` Krzysztof Kozlowski
2026-08-13 7:00 ` [PATCH v4 3/5] ASoC: fsl_asrc/fsl_easrc: move DMA params into pair/context struct shengjiu.wang
2026-08-13 7:09 ` sashiko-bot
2026-08-13 16:54 ` Frank Li [this message]
2026-08-13 16:52 ` Frank Li
2026-08-13 7:00 ` [PATCH v4 4/5] ASoC: fsl_asrc: expose individual DAIs per conversion path shengjiu.wang
2026-08-13 7:14 ` sashiko-bot
2026-08-13 7:00 ` [PATCH v4 5/5] ASoC: fsl_easrc: " shengjiu.wang
2026-08-13 7:10 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=an32wu6oTkCAwoe1@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=imx@lists.linux.dev \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=shengjiu.wang@oss.nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.