| Message ID | 20181010103606.GA14563@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | udlfb: fix some inconsistent NULL checking | expand |
[ added Mikulas, Alexey, Colin & Wen to Cc: ] On 10/10/2018 12:36 PM, Dan Carpenter wrote: > In the current kernel, then kzalloc() can't fail for small allocations, > but if it did fail then we would have a NULL dereference in the error > handling. Also in dlfb_usb_disconnect() if "info" were NULL then it > would cause an Oops inside the unregister_framebuffer() function but it > can't be NULL so let's remove that check. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Patch queued for 4.21, thanks (also sorry for the delay). [ Mikulas, Alexey, Colin & Wen: please verify that there are no known issues with udlfb left with this patch applied (thanks!). ] > diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c > index 070026a7e55a..1d034dddc556 100644 > --- a/drivers/video/fbdev/udlfb.c > +++ b/drivers/video/fbdev/udlfb.c > @@ -1598,7 +1598,7 @@ static int dlfb_usb_probe(struct usb_interface *intf, > dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL); > if (!dlfb) { > dev_err(&intf->dev, "%s: failed to allocate dlfb\n", __func__); > - goto error; > + return -ENOMEM; > } > > INIT_LIST_HEAD(&dlfb->deferred_free); > @@ -1703,7 +1703,7 @@ static int dlfb_usb_probe(struct usb_interface *intf, > error: > if (dlfb->info) { > dlfb_ops_destroy(dlfb->info); > - } else if (dlfb) { > + } else { > usb_put_dev(dlfb->udev); > kfree(dlfb); > } > @@ -1730,12 +1730,10 @@ static void dlfb_usb_disconnect(struct usb_interface *intf) > /* this function will wait for all in-flight urbs to complete */ > dlfb_free_urb_list(dlfb); > > - if (info) { > - /* remove udlfb's sysfs interfaces */ > - for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) > - device_remove_file(info->dev, &fb_device_attrs[i]); > - device_remove_bin_file(info->dev, &edid_attr); > - } > + /* remove udlfb's sysfs interfaces */ > + for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) > + device_remove_file(info->dev, &fb_device_attrs[i]); > + device_remove_bin_file(info->dev, &edid_attr); > > unregister_framebuffer(info); > } Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c index 070026a7e55a..1d034dddc556 100644 --- a/drivers/video/fbdev/udlfb.c +++ b/drivers/video/fbdev/udlfb.c @@ -1598,7 +1598,7 @@ static int dlfb_usb_probe(struct usb_interface *intf, dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL); if (!dlfb) { dev_err(&intf->dev, "%s: failed to allocate dlfb\n", __func__); - goto error; + return -ENOMEM; } INIT_LIST_HEAD(&dlfb->deferred_free); @@ -1703,7 +1703,7 @@ static int dlfb_usb_probe(struct usb_interface *intf, error: if (dlfb->info) { dlfb_ops_destroy(dlfb->info); - } else if (dlfb) { + } else { usb_put_dev(dlfb->udev); kfree(dlfb); } @@ -1730,12 +1730,10 @@ static void dlfb_usb_disconnect(struct usb_interface *intf) /* this function will wait for all in-flight urbs to complete */ dlfb_free_urb_list(dlfb); - if (info) { - /* remove udlfb's sysfs interfaces */ - for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) - device_remove_file(info->dev, &fb_device_attrs[i]); - device_remove_bin_file(info->dev, &edid_attr); - } + /* remove udlfb's sysfs interfaces */ + for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) + device_remove_file(info->dev, &fb_device_attrs[i]); + device_remove_bin_file(info->dev, &edid_attr); unregister_framebuffer(info); }
In the current kernel, then kzalloc() can't fail for small allocations, but if it did fail then we would have a NULL dereference in the error handling. Also in dlfb_usb_disconnect() if "info" were NULL then it would cause an Oops inside the unregister_framebuffer() function but it can't be NULL so let's remove that check. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>