| Message ID | 20221122123523.3068034-4-john@metanate.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 944fe915d00d3cb1bacb1e77cabfb6dc82e6f8b8 |
| Headers | show |
| Series | usb: gadget: f_hid: fix f_hidg lifetime vs cdev | expand |
W dniu 22.11.2022 o 13:35, John Keeping pisze: > Unify error handling at the end of the function, reducing the risk of > missing something on one of the error paths. > > Moving the increment of opts->refcnt later means there is no need to > decrement it on the error path and is safe as this is guarded by > opts->lock which is held for this entire section. > > Signed-off-by: John Keeping <john@metanate.com> Reviewed-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/usb/gadget/function/f_hid.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c > index 6be6009f911e..a8da3b4a2855 100644 > --- a/drivers/usb/gadget/function/f_hid.c > +++ b/drivers/usb/gadget/function/f_hid.c > @@ -1269,18 +1269,14 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) > opts = container_of(fi, struct f_hid_opts, func_inst); > > mutex_lock(&opts->lock); > - ++opts->refcnt; > > device_initialize(&hidg->dev); > hidg->dev.release = hidg_release; > hidg->dev.class = hidg_class; > hidg->dev.devt = MKDEV(major, opts->minor); > ret = dev_set_name(&hidg->dev, "hidg%d", opts->minor); > - if (ret) { > - --opts->refcnt; > - mutex_unlock(&opts->lock); > - return ERR_PTR(ret); > - } > + if (ret) > + goto err_unlock; > > hidg->bInterfaceSubClass = opts->subclass; > hidg->bInterfaceProtocol = opts->protocol; > @@ -1291,14 +1287,13 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) > opts->report_desc_length, > GFP_KERNEL); > if (!hidg->report_desc) { > - put_device(&hidg->dev); > - --opts->refcnt; > - mutex_unlock(&opts->lock); > - return ERR_PTR(-ENOMEM); > + ret = -ENOMEM; > + goto err_put_device; > } > } > hidg->use_out_ep = !opts->no_out_endpoint; > > + ++opts->refcnt; > mutex_unlock(&opts->lock); > > hidg->func.name = "hid"; > @@ -1313,6 +1308,12 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) > hidg->qlen = 4; > > return &hidg->func; > + > +err_put_device: > + put_device(&hidg->dev); > +err_unlock: > + mutex_unlock(&opts->lock); > + return ERR_PTR(ret); > } > > DECLARE_USB_FUNCTION_INIT(hid, hidg_alloc_inst, hidg_alloc);
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index 6be6009f911e..a8da3b4a2855 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -1269,18 +1269,14 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) opts = container_of(fi, struct f_hid_opts, func_inst); mutex_lock(&opts->lock); - ++opts->refcnt; device_initialize(&hidg->dev); hidg->dev.release = hidg_release; hidg->dev.class = hidg_class; hidg->dev.devt = MKDEV(major, opts->minor); ret = dev_set_name(&hidg->dev, "hidg%d", opts->minor); - if (ret) { - --opts->refcnt; - mutex_unlock(&opts->lock); - return ERR_PTR(ret); - } + if (ret) + goto err_unlock; hidg->bInterfaceSubClass = opts->subclass; hidg->bInterfaceProtocol = opts->protocol; @@ -1291,14 +1287,13 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) opts->report_desc_length, GFP_KERNEL); if (!hidg->report_desc) { - put_device(&hidg->dev); - --opts->refcnt; - mutex_unlock(&opts->lock); - return ERR_PTR(-ENOMEM); + ret = -ENOMEM; + goto err_put_device; } } hidg->use_out_ep = !opts->no_out_endpoint; + ++opts->refcnt; mutex_unlock(&opts->lock); hidg->func.name = "hid"; @@ -1313,6 +1308,12 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) hidg->qlen = 4; return &hidg->func; + +err_put_device: + put_device(&hidg->dev); +err_unlock: + mutex_unlock(&opts->lock); + return ERR_PTR(ret); } DECLARE_USB_FUNCTION_INIT(hid, hidg_alloc_inst, hidg_alloc);
Unify error handling at the end of the function, reducing the risk of missing something on one of the error paths. Moving the increment of opts->refcnt later means there is no need to decrement it on the error path and is safe as this is guarded by opts->lock which is held for this entire section. Signed-off-by: John Keeping <john@metanate.com> --- drivers/usb/gadget/function/f_hid.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)