diff mbox series

[24/25] dmaengine: dw-edma: Skip cleanup procedure if no private data found

Message ID 20220324014836.19149-25-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Superseded
Headers show
Series dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand

Commit Message

Serge Semin March 24, 2022, 1:48 a.m. UTC
DW eDMA driver private data is preserved in the passed DW eDMA chip info
structure. If either probe procedure failed or for some reason the passed
info object doesn't have private data pointer initialized we need to halt
the DMA device cleanup procedure in order to prevent possible system
crashes.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Serge Semin April 18, 2022, 1:48 p.m. UTC | #1
On Fri, Mar 25, 2022 at 11:45:03PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Mar 24, 2022 at 04:48:35AM +0300, Serge Semin wrote:
> > DW eDMA driver private data is preserved in the passed DW eDMA chip info
> > structure. If either probe procedure failed or for some reason the passed
> > info object doesn't have private data pointer initialized we need to halt
> > the DMA device cleanup procedure in order to prevent possible system
> > crashes.
> > 
> 

> How come remove() could happen when probe() failed? If you hit this issue then
> something else is utterly going wrong.

It fully depends on the DW eDMA client driver implementation, which
can't and in general shouldn't be guessed. But what must be done in
the DW eDMA driver is a protection against the invalid data being
passed to the exported API methods. That wrong situation must be
detected and handled in the API user code. It's much easier to do by
having an error code returned from the dw_edma_remove() method than
catching random system crashes.

-Sergey

> 
> Thanks,
> Mani
> 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index ca5cd7c99571..b932682a8ba8 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -1030,6 +1030,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> >  	struct dw_edma *dw = chip->dw;
> >  	int i;
> >  
> > +	/* Skip removal if no private data found */
> > +	if (!dw)
> > +		return -ENODEV;
> > +
> >  	/* Disable eDMA */
> >  	dw_edma_v0_core_off(dw);
> >  
> > -- 
> > 2.35.1
> >
Manivannan Sadhasivam April 23, 2022, 2:45 p.m. UTC | #2
On Mon, Apr 18, 2022 at 04:48:33PM +0300, Serge Semin wrote:
> On Fri, Mar 25, 2022 at 11:45:03PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Mar 24, 2022 at 04:48:35AM +0300, Serge Semin wrote:
> > > DW eDMA driver private data is preserved in the passed DW eDMA chip info
> > > structure. If either probe procedure failed or for some reason the passed
> > > info object doesn't have private data pointer initialized we need to halt
> > > the DMA device cleanup procedure in order to prevent possible system
> > > crashes.
> > > 
> > 
> 
> > How come remove() could happen when probe() failed? If you hit this issue then
> > something else is utterly going wrong.
> 
> It fully depends on the DW eDMA client driver implementation, which
> can't and in general shouldn't be guessed. But what must be done in
> the DW eDMA driver is a protection against the invalid data being
> passed to the exported API methods. That wrong situation must be
> detected and handled in the API user code. It's much easier to do by
> having an error code returned from the dw_edma_remove() method than
> catching random system crashes.
> 

Even though I don't see any practical issue, I'm fine with this.

Thanks,
Mani

> -Sergey
> 
> > 
> > Thanks,
> > Mani
> > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index ca5cd7c99571..b932682a8ba8 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -1030,6 +1030,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> > >  	struct dw_edma *dw = chip->dw;
> > >  	int i;
> > >  
> > > +	/* Skip removal if no private data found */
> > > +	if (!dw)
> > > +		return -ENODEV;
> > > +
> > >  	/* Disable eDMA */
> > >  	dw_edma_v0_core_off(dw);
> > >  
> > > -- 
> > > 2.35.1
> > >
diff mbox series

Patch

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index ca5cd7c99571..b932682a8ba8 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -1030,6 +1030,10 @@  int dw_edma_remove(struct dw_edma_chip *chip)
 	struct dw_edma *dw = chip->dw;
 	int i;
 
+	/* Skip removal if no private data found */
+	if (!dw)
+		return -ENODEV;
+
 	/* Disable eDMA */
 	dw_edma_v0_core_off(dw);