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]> > >