On Mon, Feb 15, 2016 at 09:33:11PM -0500, Insu Yun wrote:
> Since rc_allocate_device() uses kmalloc,
> it can returns NULL, so need to check,
> otherwise, NULL derefenrece can be happened.
Thanks for catching that.
> Signed-off-by: Insu Yun <wuninsu@xxxxxxxxx>
> ---
> drivers/media/rc/igorplugusb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
> index b36e515..df37cd5 100644
> --- a/drivers/media/rc/igorplugusb.c
> +++ b/drivers/media/rc/igorplugusb.c
> @@ -191,6 +191,8 @@ static int igorplugusb_probe(struct usb_interface *intf,
> usb_make_path(udev, ir->phys, sizeof(ir->phys));
>
> rc = rc_allocate_device();
> + if (!rc)
> + goto fail;
At this point, ret is not initialized but will be used in the error path.
> rc->input_name = DRIVER_DESC;
> rc->input_phys = ir->phys;
> usb_to_input_id(udev, &rc->input_id);
> @@ -213,6 +215,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
> ir->rc = rc;
> ret = rc_register_device(rc);
> if (ret) {
I'm not sure how common it is to goto into another nesting level for an
error path. Also I just noticed that the code is leaking the timer in
the error path.
It might be better to put the "fail:" at the end after the last return
for the successful case, and have a goto to it after both
rc_allocate_device() and rc_register_device() in case they fail.
> +fail:
> dev_err(&intf->dev, "failed to register rc device: %d", ret);
> rc_free_device(rc);
> usb_free_urb(ir->urb);
> --
> 1.9.1
Thanks
Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
