lkml.org 
[lkml]   [2019]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] sh: dma: Add missing IS_ERR test


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

\
 
 \ /
  Last update: 2019-06-11 12:20    [W:0.071 / U:1.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site