Re: [PATCH v1 2/7] ASoC: sprd: Replace dma_request_slave_channel() by dma_request_chan()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: sprd: crash in dma_release_channel() due to ERR_PTR leak
--

commit 5e4c398ebe4c03b68c30e555198977e4ad4905e4
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

ASoC: sprd: Replace dma_request_slave_channel() by dma_request_chan()

This commit replaces deprecated dma_request_slave_channel calls with
dma_request_chan. It updates the error checking from NULL checks to
IS_ERR.

> diff --git a/sound/soc/sprd/sprd-pcm-compress.c b/sound/soc/sprd/sprd-pcm-compress.c
> index e5249924b54d9..5613c25fffd73 100644
> --- a/sound/soc/sprd/sprd-pcm-compress.c
> +++ b/sound/soc/sprd/sprd-pcm-compress.c
> @@ -153,11 +153,10 @@ static int sprd_platform_compr_dma_config(struct snd_soc_component *component,
>  		return -EINVAL;
>  	}
>  
> -	dma->chan = dma_request_slave_channel(dev,
> -					      dma_params->chan_name[channel]);
> -	if (!dma->chan) {
> +	dma->chan = dma_request_chan(dev, dma_params->chan_name[channel]);
> +	if (IS_ERR(dma->chan)) {

[Severity: High]
If dma_request_chan fails, does this leak the error pointer into the compress
stream's persistent dma array?

When the file descriptor is closed or cleaned up, the free path in
sprd_platform_compr_free() will attempt to release the channel:

    if (dma->chan) {
        dma_release_channel(dma->chan);
        dma->chan = NULL;
    }

Since the error path doesn't clear the error pointer from dma->chan, the if
check evaluates to true. Will dma_release_channel() then dereference the error
pointer and crash?

>  		dev_err(dev, "failed to request dma channel\n");
> -		return -ENODEV;
> +		return PTR_ERR(dma->chan);
>  	}
>  
>  	sgt = sg = kzalloc_objs(*sg, sg_num);
> diff --git a/sound/soc/sprd/sprd-pcm-dma.c b/sound/soc/sprd/sprd-pcm-dma.c
> index f509a4601de25..1586eff61120a 100644
> --- a/sound/soc/sprd/sprd-pcm-dma.c
> +++ b/sound/soc/sprd/sprd-pcm-dma.c
> @@ -171,13 +171,12 @@ static int sprd_pcm_request_dma_channel(struct snd_soc_component *component,
>  	for (i = 0; i < channels; i++) {
>  		struct sprd_pcm_dma_data *data = &dma_private->data[i];
>  
> -		data->chan = dma_request_slave_channel(dev,
> -						       dma_params->chan_name[i]);
> -		if (!data->chan) {
> +		data->chan = dma_request_chan(dev, dma_params->chan_name[i]);
> +		if (IS_ERR(data->chan)) {

[Severity: High]
By directly assigning the return value here, does the error pointer become
visible to the cleanup function on failure?

When dma_request_chan fails, sprd_pcm_release_dma_channel() is called
immediately below. That function loops through the channels and checks:

    if (data->chan) {
        dma_release_channel(data->chan);
    }

This condition evaluates to true for an error pointer, blindly passing it to
dma_release_channel. Could this lead to a kernel panic when the invalid pointer
is dereferenced?

>  			dev_err(dev, "failed to request dma channel:%s\n",
>  				dma_params->chan_name[i]);
>  			sprd_pcm_release_dma_channel(substream);
> -			return -ENODEV;
> +			return PTR_ERR(data->chan);
>  		}
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915100931.3593242-1-andriy.shevchenko@linux.intel.com?part=2