* [PATCH] Staging: media: atomisp: Remove duplicated argument to || @ 2017-09-22 11:20 Georgiana Chelu 2017-09-29 13:36 ` Greg Kroah-Hartman 0 siblings, 1 reply; 4+ messages in thread From: Georgiana Chelu @ 2017-09-22 11:20 UTC (permalink / raw) To: outreachy-kernel; +Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman get_user() copies a variable from user space to kernel space put_user() copies a variable from kenel space to user space. Remove duplicated argument to || because consecutive calls of these functions with the same arguments will have the same result. Issue found by coccinelle script. Signed-off-by: Georgiana Chelu <georgiana.chelu93@gmail.com> --- drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c index 0592ac1..a27ecbe 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c @@ -432,8 +432,6 @@ static int get_atomisp_overlay32(struct atomisp_overlay *kp, &up->blend_overlay_perc_u) || get_user(kp->blend_overlay_perc_v, &up->blend_overlay_perc_v) || - get_user(kp->blend_overlay_perc_u, - &up->blend_overlay_perc_u) || get_user(kp->overlay_start_x, &up->overlay_start_y)) return -EFAULT; @@ -460,8 +458,6 @@ static int put_atomisp_overlay32(struct atomisp_overlay *kp, &up->blend_overlay_perc_u) || put_user(kp->blend_overlay_perc_v, &up->blend_overlay_perc_v) || - put_user(kp->blend_overlay_perc_u, - &up->blend_overlay_perc_u) || put_user(kp->overlay_start_x, &up->overlay_start_y)) return -EFAULT; -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: media: atomisp: Remove duplicated argument to || 2017-09-22 11:20 [PATCH] Staging: media: atomisp: Remove duplicated argument to || Georgiana Chelu @ 2017-09-29 13:36 ` Greg Kroah-Hartman 2017-10-01 10:47 ` Georgiana Chelu 0 siblings, 1 reply; 4+ messages in thread From: Greg Kroah-Hartman @ 2017-09-29 13:36 UTC (permalink / raw) To: Georgiana Chelu; +Cc: outreachy-kernel, Mauro Carvalho Chehab On Fri, Sep 22, 2017 at 04:20:55AM -0700, Georgiana Chelu wrote: > get_user() copies a variable from user space to kernel space > put_user() copies a variable from kenel space to user space. > > Remove duplicated argument to || because consecutive calls > of these functions with the same arguments will have the same > result. > > Issue found by coccinelle script. > > Signed-off-by: Georgiana Chelu <georgiana.chelu93@gmail.com> > --- > drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > index 0592ac1..a27ecbe 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > @@ -432,8 +432,6 @@ static int get_atomisp_overlay32(struct atomisp_overlay *kp, > &up->blend_overlay_perc_u) || > get_user(kp->blend_overlay_perc_v, > &up->blend_overlay_perc_v) || > - get_user(kp->blend_overlay_perc_u, > - &up->blend_overlay_perc_u) || > get_user(kp->overlay_start_x, &up->overlay_start_y)) > return -EFAULT; > I don't understand, you are changing the output of the code here, right? Why is this patch ok? confused, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: media: atomisp: Remove duplicated argument to || 2017-09-29 13:36 ` Greg Kroah-Hartman @ 2017-10-01 10:47 ` Georgiana Chelu 2017-10-03 16:16 ` Greg Kroah-Hartman 0 siblings, 1 reply; 4+ messages in thread From: Georgiana Chelu @ 2017-10-01 10:47 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: outreachy-kernel, Mauro Carvalho Chehab On 29 September 2017 at 16:36, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Fri, Sep 22, 2017 at 04:20:55AM -0700, Georgiana Chelu wrote: >> get_user() copies a variable from user space to kernel space >> put_user() copies a variable from kenel space to user space. >> >> Remove duplicated argument to || because consecutive calls >> of these functions with the same arguments will have the same >> result. >> >> Issue found by coccinelle script. >> >> Signed-off-by: Georgiana Chelu <georgiana.chelu93@gmail.com> >> --- >> drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c >> index 0592ac1..a27ecbe 100644 >> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c >> @@ -432,8 +432,6 @@ static int get_atomisp_overlay32(struct atomisp_overlay *kp, >> &up->blend_overlay_perc_u) || >> get_user(kp->blend_overlay_perc_v, >> &up->blend_overlay_perc_v) || >> - get_user(kp->blend_overlay_perc_u, >> - &up->blend_overlay_perc_u) || >> get_user(kp->overlay_start_x, &up->overlay_start_y)) >> return -EFAULT; >> > > I don't understand, you are changing the output of the code here, right? > Why is this patch ok? The output of the code is the same. This patch just removes the duplicate. I know, it's hard to see it because the patch does not show the whole if statement. The get_user() function does not change the value of the variable from userspace. So, I thought that consecutive calls over the same variable will have the same result. Here is the whole if statement. The get_user is called twice for the "blend_overlay_perc_u" and "up->blend_overlay_perc_u" variables. if (!access_ok(VERIFY_READ, up, sizeof(struct atomisp_overlay32)) || get_user(frame, &up->frame) || get_user(kp->bg_y, &up->bg_y) || get_user(kp->bg_u, &up->bg_u) || get_user(kp->bg_v, &up->bg_v) || get_user(kp->blend_input_perc_y, &up->blend_input_perc_y) || get_user(kp->blend_input_perc_u, &up->blend_input_perc_u) || get_user(kp->blend_input_perc_v, &up->blend_input_perc_v) || get_user(kp->blend_overlay_perc_y, &up->blend_overlay_perc_y) || get_user(kp->blend_overlay_perc_u, &up->blend_overlay_perc_u) || get_user(kp->blend_overlay_perc_v, &up->blend_overlay_perc_v) || get_user(kp->blend_overlay_perc_u, &up->blend_overlay_perc_u) || get_user(kp->overlay_start_x, &up->overlay_start_y)) {...} The reason behind the second change in this patch is the same. The put_user() function is called twice to write the same value into the same variable from userspace. Thank you, Georgiana > > confused, > > greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: media: atomisp: Remove duplicated argument to || 2017-10-01 10:47 ` Georgiana Chelu @ 2017-10-03 16:16 ` Greg Kroah-Hartman 0 siblings, 0 replies; 4+ messages in thread From: Greg Kroah-Hartman @ 2017-10-03 16:16 UTC (permalink / raw) To: Georgiana Chelu; +Cc: outreachy-kernel, Mauro Carvalho Chehab On Sun, Oct 01, 2017 at 01:47:41PM +0300, Georgiana Chelu wrote: > On 29 September 2017 at 16:36, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > On Fri, Sep 22, 2017 at 04:20:55AM -0700, Georgiana Chelu wrote: > >> get_user() copies a variable from user space to kernel space > >> put_user() copies a variable from kenel space to user space. > >> > >> Remove duplicated argument to || because consecutive calls > >> of these functions with the same arguments will have the same > >> result. > >> > >> Issue found by coccinelle script. > >> > >> Signed-off-by: Georgiana Chelu <georgiana.chelu93@gmail.com> > >> --- > >> drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c | 4 ---- > >> 1 file changed, 4 deletions(-) > >> > >> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > >> index 0592ac1..a27ecbe 100644 > >> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_ioctl32.c > >> @@ -432,8 +432,6 @@ static int get_atomisp_overlay32(struct atomisp_overlay *kp, > >> &up->blend_overlay_perc_u) || > >> get_user(kp->blend_overlay_perc_v, > >> &up->blend_overlay_perc_v) || > >> - get_user(kp->blend_overlay_perc_u, > >> - &up->blend_overlay_perc_u) || > >> get_user(kp->overlay_start_x, &up->overlay_start_y)) > >> return -EFAULT; > >> > > > > I don't understand, you are changing the output of the code here, right? > > Why is this patch ok? > > The output of the code is the same. This patch just removes the duplicate. > I know, it's hard to see it because the patch does not show the whole > if statement. The get_user() function does not change the value > of the variable from userspace. So, I thought that consecutive calls > over the same variable will have the same result. > > Here is the whole if statement. The get_user is called twice for > the "blend_overlay_perc_u" and "up->blend_overlay_perc_u" variables. > > if (!access_ok(VERIFY_READ, up, sizeof(struct atomisp_overlay32)) || > get_user(frame, &up->frame) || > get_user(kp->bg_y, &up->bg_y) || > get_user(kp->bg_u, &up->bg_u) || > get_user(kp->bg_v, &up->bg_v) || > get_user(kp->blend_input_perc_y, &up->blend_input_perc_y) || > get_user(kp->blend_input_perc_u, &up->blend_input_perc_u) || > get_user(kp->blend_input_perc_v, &up->blend_input_perc_v) || > get_user(kp->blend_overlay_perc_y, &up->blend_overlay_perc_y) || > get_user(kp->blend_overlay_perc_u, &up->blend_overlay_perc_u) || > get_user(kp->blend_overlay_perc_v, &up->blend_overlay_perc_v) || > get_user(kp->blend_overlay_perc_u, &up->blend_overlay_perc_u) || > get_user(kp->overlay_start_x, &up->overlay_start_y)) {...} > > > The reason behind the second change in this patch is the same. > The put_user() function is called twice to write the same value > into the same variable from userspace. Ah, ok, care to put this type of info in the changelog itself to make it more obvious? thanks, greg k-h ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-03 16:16 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-22 11:20 [PATCH] Staging: media: atomisp: Remove duplicated argument to || Georgiana Chelu 2017-09-29 13:36 ` Greg Kroah-Hartman 2017-10-01 10:47 ` Georgiana Chelu 2017-10-03 16:16 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.