Re: [PATCH 3/3] ASoC: sprd: use dmaengine_prep_slave_sg_context() instead of device_prep_slave_sg()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] ASoC: sprd: double-free in sprd_platform_compr_free() [1]
--

commit 7137d268eefd6fca1c12f5ab33b970d39d707688
Author: Frank Li <Frank.Li@nxp.com>

ASoC: sprd: use dmaengine_prep_slave_sg_context() instead of device_prep_slave_sg()

This commit replaces direct calls to chan->device->device_prep_slave_sg()
with dmaengine_prep_slave_sg_context() in sprd-pcm-compress.c and
sprd-pcm-dma.c.

> diff --git a/sound/soc/sprd/sprd-pcm-compress.c b/sound/soc/sprd/sprd-pcm-compress.c
> index f790b959cd4e4..7188adfa3f402 100644
> --- a/sound/soc/sprd/sprd-pcm-compress.c
> +++ b/sound/soc/sprd/sprd-pcm-compress.c
> @@ -236,9 +236,9 @@ static int sprd_platform_compr_dma_config(struct snd_soc_component *component,
>  	 * We configure the DMA request mode, interrupt mode, channel
>  	 * mode and channel trigger mode by the flags.
>  	 */
> -	dma->desc = dma->chan->device->device_prep_slave_sg(dma->chan, sg,
> -							    sg_num, dir,
> -							    flags, &link);
> +	dma->desc = dmaengine_prep_slave_sg_context(dma->chan, sg,
> +						    sg_num, dir,
> +						    flags, &link);
>  	if (!dma->desc) {
>  		dev_err(dev, "failed to prepare slave sg\n");
>  		ret = -ENOMEM;

[Severity: Critical]
This problem wasn't introduced by this patch, but can a failure in
sprd_platform_compr_dma_config() or sprd_platform_compr_set_params()
cause a double free of dma->chan in sprd_platform_compr_free() [1]?

The error paths in both functions call dma_release_channel() without
clearing dma->chan to NULL:

sound/soc/sprd/sprd-pcm-compress.c:sprd_platform_compr_dma_config() {
	...
config_err:
	kfree(sg);
sg_err:
	dma_release_channel(dma->chan);
	return ret;
}

sound/soc/sprd/sprd-pcm-compress.c:sprd_platform_compr_set_params() {
	...
params_err:
	dma_release_channel(stream->dma[0].chan);
config_err:
	dma_release_channel(stream->dma[1].chan);
	return ret;
}

When the stream is later closed, sprd_platform_compr_free() checks
if (dma->chan) and calls dma_release_channel() a second time on the same
channel pointer.

[1] https://sashiko.dev/#/bug/linux-2a0aea76-9cc7-47d0-8767-b0cd95e62054

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-rio_sg-v1-0-bc54b4bd8c95@nxp.com?part=3