Re: [PATCH] sh: dma: Add missing IS_ERR test

[Rolf Evers-Fischer <embedded24@evers-fischer.de>]

On Sat, 8 Jun 2019, Geert Uytterhoeven wrote:
Hi Geert,

thank you for your reply and your additional findings.

> Hi Rolf,
> 
> Thanks for your patch!
> 
> On Fri, Jun 7, 2019 at 2:04 PM Rolf Evers-Fischer
> <embedded24@evers-fischer.de> wrote:
> > get_dma_channel may return ERR_PTR, so a check is added.
> 
> It may also return NULL...

Good catch. I must have missed this.

> 
> > --- a/arch/sh/drivers/dma/dma-api.c
> > +++ b/arch/sh/drivers/dma/dma-api.c
> > @@ -94,7 +94,7 @@ int get_dma_residue(unsigned int chan)
> >         struct dma_info *info = get_dma_info(chan);
> >         struct dma_channel *channel = get_dma_channel(chan);
> >
> > -       if (info->ops->get_residue)
> > +       if (!IS_ERR(channel) && (info->ops->get_residue))
> >                 return info->ops->get_residue(channel);
> 
> ... in which case .get_residue() may crash, as some implementations
> dereference the passed channel pointer.
> 
> Hence !IS_ERR_OR_NULL()?

Yes, in fact. IS_ERR_OR_NULL is the better choice here. 
I will resend a reworked patch immediately.

> 
> I didn't check the other callers.

Well, I did. And I found that none of the implementations checks the 
passed pointer. However, no in-tree driver is using the .extend() op, but 
as long as we don't know, if any out-of-tree drivers are using it without 
any additional check, I would prefer to check for NULL or error in 
dma_extend() as well.

Kind regards,
 Rolf