Hi,
"Subject: " at the beginning of your email subject looks unnecessary.
When you're sending next version of your patch, you should add subject
prefix "[PATCH vN]", when N is number of version (eg. [PATCH v2]). See
Documentation/SubmittingPatches.
('git format-patch --subject-prefix' is helpful).
On 10/13/2014 11:35 AM, pavi1729 Sid wrote:
> Removed the usb_free_all_descriptors call in *_bind functions
> as this call is already present in usb_assign_descriptors.
> usb_assign_descriptors is the only call where usb descriptor
> allocation happens, also in case of error freeing of the
> allocated memory takes place in the same call. Hence the
> call in the *_bind functions are redundant.
> Also the presence of usb_free_all_descriptors in the *_bind
> functions might result in double-free corruption of the
> allocated mmeory.
s/mmeory/memory
>
> Signed-off-by: Pavitrakumar Managutte <pavitra1729@xxxxxxxxx>
> ---
> drivers/usb/gadget/function/f_eem.c | 1 -
> drivers/usb/gadget/function/f_hid.c | 7 +++----
> drivers/usb/gadget/function/f_ncm.c | 1 -
> drivers/usb/gadget/function/f_phonet.c | 1 -
> drivers/usb/gadget/function/f_rndis.c | 5 +++--
> drivers/usb/gadget/function/f_subset.c | 1 -
> drivers/usb/gadget/function/f_uac2.c | 10 ++++++----
> 7 files changed, 12 insertions(+), 14 deletions(-)
There is another one usb function which needs to be fixed this way:
drivers/usb/gadget/function/f_obex.c
>
> diff --git a/drivers/usb/gadget/function/f_eem.c
> b/drivers/usb/gadget/function/f_eem.c
> index 4d8b236..c9e90de 100644
> --- a/drivers/usb/gadget/function/f_eem.c
> +++ b/drivers/usb/gadget/function/f_eem.c
> @@ -325,7 +325,6 @@ static int eem_bind(struct usb_configuration *c,
> struct usb_function *f)
> return 0;
>
> fail:
> - usb_free_all_descriptors(f);
> if (eem->port.out_ep)
> eem->port.out_ep->driver_data = NULL;
> if (eem->port.in_ep)
> diff --git a/drivers/usb/gadget/function/f_hid.c
> b/drivers/usb/gadget/function/f_hid.c
> index a95290a..c36de0c 100644
> --- a/drivers/usb/gadget/function/f_hid.c
> +++ b/drivers/usb/gadget/function/f_hid.c
> @@ -621,12 +621,14 @@ static int __init hidg_bind(struct
> usb_configuration *c, struct usb_function *f)
> dev = MKDEV(major, hidg->minor);
> status = cdev_add(&hidg->cdev, dev, 1);
> if (status)
> - goto fail;
> + goto fail_free_descs;
>
> device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
>
> return 0;
>
> +fail_free_descs:
> + usb_free_all_descriptors(f);
> fail:
> ERROR(f->config->cdev, "hidg_bind FAILED\n");
> if (hidg->req != NULL) {
> @@ -635,7 +637,6 @@ fail:
> usb_ep_free_request(hidg->in_ep, hidg->req);
> }
>
> - usb_free_all_descriptors(f);
> return status;
> }
>
> @@ -652,8 +653,6 @@ static void hidg_unbind(struct usb_configuration
> *c, struct usb_function *f)
> kfree(hidg->req->buf);
> usb_ep_free_request(hidg->in_ep, hidg->req);
>
> - usb_free_all_descriptors(f);
> -
Why usb_free_all_descriptors() removed from here?
> kfree(hidg->report_desc);
> kfree(hidg);
> }
> 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);
> 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_phonet.c
> b/drivers/usb/gadget/function/f_phonet.c
> index b9cfc15..74ddcac 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -571,7 +571,6 @@ err_req:
> for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
> usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
> err:
> - usb_free_all_descriptors(f);
What if usb_ep_alloc_request() fails?
Consider calling usb_free_all_descriptors() under label "err_req".
> if (fp->out_ep)
> fp->out_ep->driver_data = NULL;
> if (fp->in_ep)
> 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);
> diff --git a/drivers/usb/gadget/function/f_subset.c
> b/drivers/usb/gadget/function/f_subset.c
> index 1ea8baf..e3dfa67 100644
> --- a/drivers/usb/gadget/function/f_subset.c
> +++ b/drivers/usb/gadget/function/f_subset.c
> @@ -380,7 +380,6 @@ geth_bind(struct usb_configuration *c, struct
> usb_function *f)
> return 0;
>
> fail:
> - usb_free_all_descriptors(f);
> /* we might as well release our claims on endpoints */
> if (geth->port.out_ep)
> geth->port.out_ep->driver_data = NULL;
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 3ed89ec..b45c39c 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -994,7 +994,7 @@ afunc_bind(struct usb_configuration *cfg, struct
> usb_function *fn)
> prm->rbuf = kzalloc(prm->max_psize * USB_XFERS, GFP_KERNEL);
> if (!prm->rbuf) {
> prm->max_psize = 0;
> - goto err;
> + goto err_free_descs;
> }
>
> prm = &agdev->uac2.p_prm;
> @@ -1002,17 +1002,19 @@ afunc_bind(struct usb_configuration *cfg,
> struct usb_function *fn)
> prm->rbuf = kzalloc(prm->max_psize * USB_XFERS, GFP_KERNEL);
> if (!prm->rbuf) {
> prm->max_psize = 0;
> - goto err;
> + goto err_free_descs;
> }
>
> ret = alsa_uac2_init(agdev);
> if (ret)
> - goto err;
> + goto err_free_descs;
> return 0;
> +
> +err_free_descs:
> + usb_free_all_descriptors(fn);
> err:
> kfree(agdev->uac2.p_prm.rbuf);
> kfree(agdev->uac2.c_prm.rbuf);
> - usb_free_all_descriptors(fn);
> if (agdev->in_ep)
> agdev->in_ep->driver_data = NULL;
> if (agdev->out_ep)
>
Thanks,
Robert Baldyga
--
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
