lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 29 Jul 2020 18:13:44 +0300 From: Andy Shevchenko <andy.shevchenko@...il.com> To: Cengiz Can <cengiz@...nel.wtf> Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, Sakari Ailus <sakari.ailus@...ux.intel.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Linux Media Mailing List <linux-media@...r.kernel.org>, "open list:STAGING SUBSYSTEM" <devel@...verdev.osuosl.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] staging: atomisp: move null check to earlier point On Wed, Jul 29, 2020 at 5:00 PM Cengiz Can <cengiz@...nel.wtf> wrote: > > `find_gmin_subdev` function that returns a pointer to `struct > gmin_subdev` can return NULL. > > In `gmin_v2p8_ctrl` there's a call to this function but the possibility > of a NULL was not checked before its being dereferenced. ie: > > ``` > /* Acquired here --------v */ > struct gmin_subdev *gs = find_gmin_subdev(subdev); > int ret; > int value; > > /* v------Dereferenced here */ > if (gs->v2p8_gpio >= 0) { > pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n", > gs->v2p8_gpio); > ret = gpio_request(gs->v2p8_gpio, "camera_v2p8"); > if (!ret) > ret = gpio_direction_output(gs->v2p8_gpio, 0); > if (ret) > pr_err("V2P8 GPIO initialization failed\n"); > } > ``` > > I have moved the NULL check before deref point. "Move the NULL check..." See Submitting Patches documentation how to avoid "This patch", "I", "we", etc. > diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c > index 0df46a1af5f0..8e9c5016f299 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c > +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c > @@ -871,6 +871,11 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on) > int ret; > int value; > > + if (!gs) { > + pr_err("Unable to find gmin subdevice\n"); > + return -EINVAL; And here is a change of semantics... > + } ... > - if (!gs || gs->v2p8_on == on) > + if (gs->v2p8_on == on) > return 0; ...compared to above. -- With Best Regards, Andy Shevchenko
Powered by blists - more mailing lists