Re: [PATCH 03/21] media: davinci_vpfe: fix vpfe_ipipe_init() error handling

2018-10-11 Thread Joel Fernandes 
On Mon, Oct 08, 2018 at 09:46:01PM -0700, Joel Fernandes wrote:
> On Fri, Apr 06, 2018 at 10:23:04AM -0400, Mauro Carvalho Chehab wrote:
> > As warned:
> > drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1834 vpfe_ipipe_init() 
> > error: we previously assumed 'res' could be null (see line 1797)
> > 
> > There's something wrong at vpfe_ipipe_init():
> > 
> > 1) it caches the resourse_size() from from the first region
> >and reuses to the second region;
> > 
> > 2) the "res" var is overriden 3 times;
> > 
> > 3) at free logic, it assumes that "res->start" is not
> >overriden by platform_get_resource(pdev, IORESOURCE_MEM, 6),
> >but that's not true, as it can even be NULL there.
> > 
> > This patch fixes the above issues by:
> > 
> > a) store the resources used by release_mem_region() on
> >a separate var;
> > 
> > b) stop caching resource_size(), using the function where
> >needed.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> I ran coccicheck on a 4.14.74 stable kernel and noticed that 'res' can be
> NULL in vpfe_ipipe_init. It looks like this patch is not included in the 4.14
> stable series. Can this patch be applied? I applied it myself and it applies
> cleanly, but I have no way to test it.
> 
> That 'res->start' error_release could end up a NULL pointer deref.

Should this patch goto 4.14 stable? Seems straightforward and worth it to
prevent the possible NULL pointer deref issue.

 - Joel

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/21] media: davinci_vpfe: fix vpfe_ipipe_init() error handling

2018-10-08 Thread Joel Fernandes 
On Fri, Apr 06, 2018 at 10:23:04AM -0400, Mauro Carvalho Chehab wrote:
> As warned:
>   drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1834 vpfe_ipipe_init() 
> error: we previously assumed 'res' could be null (see line 1797)
> 
> There's something wrong at vpfe_ipipe_init():
> 
> 1) it caches the resourse_size() from from the first region
>and reuses to the second region;
> 
> 2) the "res" var is overriden 3 times;
> 
> 3) at free logic, it assumes that "res->start" is not
>overriden by platform_get_resource(pdev, IORESOURCE_MEM, 6),
>but that's not true, as it can even be NULL there.
> 
> This patch fixes the above issues by:
> 
> a) store the resources used by release_mem_region() on
>a separate var;
> 
> b) stop caching resource_size(), using the function where
>needed.
> 
> Signed-off-by: Mauro Carvalho Chehab 

I ran coccicheck on a 4.14.74 stable kernel and noticed that 'res' can be
NULL in vpfe_ipipe_init. It looks like this patch is not included in the 4.14
stable series. Can this patch be applied? I applied it myself and it applies
cleanly, but I have no way to test it.

That 'res->start' error_release could end up a NULL pointer deref.

 - Joel

 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/21] media: davinci_vpfe: fix vpfe_ipipe_init() error handling

2018-04-06 Thread Mauro Carvalho Chehab 
As warned:
drivers/staging/media/davinci_vpfe/dm365_ipipe.c:1834 vpfe_ipipe_init() 
error: we previously assumed 'res' could be null (see line 1797)

There's something wrong at vpfe_ipipe_init():

1) it caches the resourse_size() from from the first region
   and reuses to the second region;

2) the "res" var is overriden 3 times;

3) at free logic, it assumes that "res->start" is not
   overriden by platform_get_resource(pdev, IORESOURCE_MEM, 6),
   but that's not true, as it can even be NULL there.

This patch fixes the above issues by:

a) store the resources used by release_mem_region() on
   a separate var;

b) stop caching resource_size(), using the function where
   needed.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index dd0cfc79f50c..b3a193ddd778 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1778,25 +1778,25 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct 
platform_device *pdev)
struct media_pad *pads = >pads[0];
struct v4l2_subdev *sd = >subdev;
struct media_entity *me = >entity;
-   static resource_size_t  res_len;
-   struct resource *res;
+   struct resource *res, *memres;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
if (!res)
return -ENOENT;
 
-   res_len = resource_size(res);
-   res = request_mem_region(res->start, res_len, res->name);
-   if (!res)
+   memres = request_mem_region(res->start, resource_size(res), res->name);
+   if (!memres)
return -EBUSY;
-   ipipe->base_addr = ioremap_nocache(res->start, res_len);
+   ipipe->base_addr = ioremap_nocache(memres->start,
+  resource_size(memres));
if (!ipipe->base_addr)
goto error_release;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 6);
if (!res)
goto error_unmap;
-   ipipe->isp5_base_addr = ioremap_nocache(res->start, res_len);
+   ipipe->isp5_base_addr = ioremap_nocache(res->start,
+   resource_size(res));
if (!ipipe->isp5_base_addr)
goto error_unmap;
 
@@ -1831,7 +1831,7 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct 
platform_device *pdev)
 error_unmap:
iounmap(ipipe->base_addr);
 error_release:
-   release_mem_region(res->start, res_len);
+   release_mem_region(memres->start, resource_size(memres));
return -ENOMEM;
 }
 
-- 
2.14.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel