* pavi1729 | 2014-10-15 14:47:53 [+0530]:
>Removed usb_free_all_descriptors in the bind functions, which
>results in double-free corruption of the descriptors on error path.
>The usb descriptors are allocated by usb_assign_descriptors.
>
>Signed-off-by: Pavitrakumar Managutte <pavitra1729@xxxxxxxxx>
>Reviewed-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
You can add
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
*but* the patch is badly formated, you need to fix this.
>diff --git a/drivers/usb/gadget/function/f_ncm.c
>b/drivers/usb/gadget/function/f_ncm.c
>index bcdc882..38a9279 100644
>--- a/drivers/usb/gadget/function/f_ncm.c
>+++ b/drivers/usb/gadget/function/f_ncm.c
>@@ -1453,7 +1453,6 @@ static int ncm_bind(struct usb_configuration *c,
>struct usb_function *f)
> return 0;
>
> fail:
>- usb_free_all_descriptors(f);
This is okay, but if you scroll up, you will notice that
usb_assign_descriptors() isn't checked. Could you please add this as a
separate patch?
> if (ncm->notify_req) {
> kfree(ncm->notify_req->buf);
> usb_ep_free_request(ncm->notify, ncm->notify_req);
>diff --git a/drivers/usb/gadget/function/f_rndis.c
>b/drivers/usb/gadget/function/f_rndis.c
>index ddb09dc..2f0517f 100644
>--- a/drivers/usb/gadget/function/f_rndis.c
>+++ b/drivers/usb/gadget/function/f_rndis.c
>@@ -803,7 +803,7 @@ rndis_bind(struct usb_configuration *c, struct
>usb_function *f)
> if (rndis->manufacturer && rndis->vendorID &&
> rndis_set_param_vendor(rndis->config, rndis->vendorID,
> rndis->manufacturer))
>- goto fail;
>+ goto fail_free_descs;
>
> /* NOTE: all that is done without knowing or caring about
> * the network link ... which is unavailable to this code
>@@ -817,10 +817,11 @@ rndis_bind(struct usb_configuration *c, struct
>usb_function *f)
> rndis->notify->name);
> return 0;
>
>+fail_free_descs:
>+ usb_free_all_descriptors(f);
> fail:
> kfree(f->os_desc_table);
> f->os_desc_n = 0;
>- usb_free_all_descriptors(f);
>
> if (rndis->notify_req) {
> kfree(rndis->notify_req->buf);
The remaining issue is that if you get here via fail_free_descs you
return the error path with status == 0 and so the caller does not notice
that somthing went wrong. Would you please fix this (in a separate
patch)?
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
