On 10/13/2014 04:25 PM, 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 and in case of error, freeing of the
> allocated memory takes place inside the same call. Hence the
> call in the *_bind functions are redundant.
> The presence of usb_free_all_descriptors in the *_bind
> functions might result in double-free corruption of the
> allocated memory.
You should update your commit message since it does not describe what
exactly you do.
Patch doesn't apply to latest next tree. You should rebase it, otherwise
maintainer wouldn't accept it.
>
> Signed-off-by: Pavitrakumar Managutte <pavitra1729@xxxxxxxxx>
Code looks good for me. You can add my reviewed-by under your sign-off.
Reviewed-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/f_eem.c | 1 -
> drivers/usb/gadget/function/f_hid.c | 5 +++--
> drivers/usb/gadget/function/f_ncm.c | 1 -
> drivers/usb/gadget/function/f_obex.c | 1 -
> drivers/usb/gadget/function/f_phonet.c | 2 +-
> drivers/usb/gadget/function/f_rndis.c | 5 +++--
> drivers/usb/gadget/function/f_subset.c | 1 -
> drivers/usb/gadget/function/f_uac2.c | 10 ++++++----
> 8 files changed, 13 insertions(+), 13 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..59ab62c 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;
> }
>
> 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_obex.c
> b/drivers/usb/gadget/function/f_obex.c
> index aebae18..9f7148d 100644
> --- a/drivers/usb/gadget/function/f_obex.c
> +++ b/drivers/usb/gadget/function/f_obex.c
> @@ -391,7 +391,6 @@ static int obex_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 (obex->port.out)
> obex->port.out->driver_data = NULL;
> diff --git a/drivers/usb/gadget/function/f_phonet.c
> b/drivers/usb/gadget/function/f_phonet.c
> index b9cfc15..1ec8b7f 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -570,8 +570,8 @@ static int pn_bind(struct usb_configuration *c,
> struct usb_function *f)
> 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);
> +err:
> 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)
>
--
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
