Re: [PATCH v3] dmaengine: qcom: gpi: Fix resource leaks as part of channel clean up

Praveen Talari <[email protected]>
Newsgroups org.kernel.vger.dmaengine,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 19-08-2026 17:09, Aniket Randive wrote:
> The gpi_ch_init() error handling paths do not fully clean up resources
> when channel initialization fails.
>
> The unwind paths iterate over the original channel pointer instead of
> the channels stored in gpii->gchan[], preventing previously initialized
> sibling channels from being properly reset and deallocated.
>
> In addition, gpi_alloc_chan_resources() allocates ch_ring before calling
> gpi_ch_init(), but does not release it when initialization fails,
> resulting in a memory leak.
>
> The event ring cleanup path also leaves the PM state unchanged after
> freeing the ring. As a result, subsequent cleanup may incorrectly assume
> that the ring is still allocated, leading to a redundant EV_CMD_DEALLOC
> command and an attempt to deallocate an already released ring.
>
> Fix the unwind paths to operate on the correct channels, release
> ch_ring when channel initialization fails, and restore the event ring
> PM state after freeing the ring.
>
> Signed-off-by: Aniket Randive <[email protected]>
> ---
> Changes in v3:
> - Updated the commit message as per Mukesh suggestion.
> - Link to v2: https://patch.msgid.link/[email protected]
>
> Changes in v2:
> - Updated the commit message as per Mukesh suggestion.
> ---
>   drivers/dma/qcom/gpi.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index a5055a6273af..c41bfac65ddf 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -1965,16 +1965,19 @@ static int gpi_ch_init(struct gchan *gchan)
>   error_start_chan:
>   	for (i = i - 1; i >= 0; i--) {
>   		gpi_stop_chan(&gpii->gchan[i]);
> -		gpi_send_cmd(gpii, gchan, GPI_CH_CMD_RESET);
> +		gpi_send_cmd(gpii, &gpii->gchan[i], GPI_CH_CMD_RESET);
>   	}
>   	i = 2;
>   error_alloc_chan:
>   	for (i = i - 1; i >= 0; i--)
> -		gpi_reset_chan(gchan, GPI_CH_CMD_DE_ALLOC);
> +		gpi_reset_chan(&gpii->gchan[i], GPI_CH_CMD_DE_ALLOC);
>   error_alloc_ev_ring:
>   	gpi_disable_interrupts(gpii);
>   error_config_int:
>   	gpi_free_ring(&gpii->ev_ring, gpii);
> +	write_lock_irq(&gpii->pm_lock);
> +	gpii->pm_state = DISABLE_STATE;
> +	write_unlock_irq(&gpii->pm_lock);
>   exit_gpi_init:
>   	return ret;
>   }
> @@ -2065,6 +2068,8 @@ static int gpi_alloc_chan_resources(struct dma_chan *chan)
>   		goto xfer_alloc_err;
>   
>   	ret = gpi_ch_init(gchan);
> +	if (ret)
> +		gpi_free_ring(&gchan->ch_ring, gpii);

Reviewed-by: Praveen Talari <[email protected]>

Thanks,

Praveen Talari

>   
>   	mutex_unlock(&gpii->ctrl_lock);
>   
>
> ---
> base-commit: 415606a7be939835db9b0d6b711887586646346d
> change-id: 20260803-gpi_bug_fix-b0b80ef315b5
>
> Best regards,
> --
> Aniket Randive <[email protected]>
>
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.