Re: [PATCH 18/21] media: isppreview: fix __user annotations

2018-04-06 Thread Mauro Carvalho Chehab
Em Fri, 06 Apr 2018 18:54:50 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Friday, 6 April 2018 17:23:19 EEST Mauro Carvalho Chehab wrote:
> > That prevent those warnings:
> >drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect
> > type in initializer (different address spaces)
> > drivers/media/platform/omap3isp/isppreview.c:893:45:expected void
> > [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45:
> >got void *[noderef] 
> > drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference
> > of noderef expression  
> 
> That's nice, but it would be even nicer to explain what the problem is and 
> how 
> you fix it, otherwise one might be left wondering if the fix is correct, or 
> if 
> it could be a false positive.

Ok. Please see the enclosed patch.

> With the commit message updated,
> 
> Reviewed-by: Laurent Pinchart 


Thanks,
Mauro

[PATCH] media: isppreview: fix __user annotations

The 'from' variable at preview_config() expects an __user * type.

However, the logic there does:

from = *(void * __user *) ((void *)cfg + attr->config_offset);

With actually means a void pointer, pointing to a void __ user
pointer. When the first pointer is de-referenced with *(foo),
the type it returns is "void *" instead of "void __user *".

Change it to:
from = *(void __user **) ((void *)cfg + attr->config_offset);

in order to obtain, when de-referenced, a void __user pointer,
as desired.

That prevent those warnings:
   drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect type 
in initializer (different address spaces)
   drivers/media/platform/omap3isp/isppreview.c:893:45:expected void 
[noderef] *from
   drivers/media/platform/omap3isp/isppreview.c:893:45:got void *[noderef] 

   drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference of 
noderef expression

Reviewed-by: Laurent Pinchart 
Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/platform/omap3isp/isppreview.c 
b/drivers/media/platform/omap3isp/isppreview.c
index ac30a0f83780..c2ef5870b231 100644
--- a/drivers/media/platform/omap3isp/isppreview.c
+++ b/drivers/media/platform/omap3isp/isppreview.c
@@ -890,7 +890,7 @@ static int preview_config(struct isp_prev_device *prev,
params = &prev->params.params[!!(active & bit)];
 
if (cfg->flag & bit) {
-   void __user *from = *(void * __user *)
+   void __user *from = *(void __user **)
((void *)cfg + attr->config_offset);
void *to = (void *)params + attr->param_offset;
size_t size = attr->param_size;



Re: [PATCH 18/21] media: isppreview: fix __user annotations

2018-04-06 Thread Laurent Pinchart
Hi Mauro,

Thank you for the patch.

On Friday, 6 April 2018 17:23:19 EEST Mauro Carvalho Chehab wrote:
> That prevent those warnings:
>drivers/media/platform/omap3isp/isppreview.c:893:45: warning: incorrect
> type in initializer (different address spaces)
> drivers/media/platform/omap3isp/isppreview.c:893:45:expected void
> [noderef] *from drivers/media/platform/omap3isp/isppreview.c:893:45:
>got void *[noderef] 
> drivers/media/platform/omap3isp/isppreview.c:893:47: warning: dereference
> of noderef expression

That's nice, but it would be even nicer to explain what the problem is and how 
you fix it, otherwise one might be left wondering if the fix is correct, or if 
it could be a false positive.

With the commit message updated,

Reviewed-by: Laurent Pinchart 

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/omap3isp/isppreview.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isppreview.c
> b/drivers/media/platform/omap3isp/isppreview.c index
> ac30a0f83780..c2ef5870b231 100644
> --- a/drivers/media/platform/omap3isp/isppreview.c
> +++ b/drivers/media/platform/omap3isp/isppreview.c
> @@ -890,7 +890,7 @@ static int preview_config(struct isp_prev_device *prev,
>   params = &prev->params.params[!!(active & bit)];
> 
>   if (cfg->flag & bit) {
> - void __user *from = *(void * __user *)
> + void __user *from = *(void __user **)
>   ((void *)cfg + attr->config_offset);
>   void *to = (void *)params + attr->param_offset;
>   size_t size = attr->param_size;

-- 
Regards,

Laurent Pinchart