Hi,
On 10/10/2014 03:09 PM, pavi1729 ivap wrote:
> From: Pavitra <pavitra1729@xxxxxxxxx>
> Date: Fri, 10 Oct 2014 16:05:30 +0530
> Subject: [PATCH] usb: gadget: function: Remove redundant
> usb_free_all_descriptors
>
> 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.
>
> Signed-off-by: Pavitra <pavitra1729@xxxxxxxxx>
You should use your full real name in sign-off (see
Documentation/SubmittingPatches).
I see there are no maintainers in your CC list. You should use
scripts/get_maintainer.pl on your patch.
> ---
> 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(-)
>
> 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..df4f390 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 err;
>
> device_create(hidg_class, NULL, dev, NULL, "%s%d", "hidg", hidg->minor);
>
> return 0;
>
> +err:
Maybe it would be better to use label name more consistent with the
naming convention in modified file (for example when existing label name
is "fail", you can use name "fail_free_descs" for your label).
> + 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);
> -
> 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);
> 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..dae8c4b 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 err;
>
> /* 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;
>
> +err:
Ditto.
> + 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..16465e3 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 fail;
> }
>
> 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 fail;
> }
>
> ret = alsa_uac2_init(agdev);
> if (ret)
> - goto err;
> + goto fail;
> return 0;
> +
> +fail:
Ditto again.
> + 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
