[PATCH] usb: gadget: function: Remove redundant usb_free_all

Discussion:

[PATCH] usb: gadget: function: Remove redundant usb_free_all_descriptors

pavi1729 ivap

2014-10-10 11:51:53 UTC

From 3272817673910c63682e8b91ce0efaef190a399a Mon Sep 17 00:00:00 2001

From: Pavitra <pavitra1729-***@public.gmane.org>
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-***@public.gmane.org>
---
drivers/usb/gadget/function/f_eem.c | 1 -
drivers/usb/gadget/function/f_hid.c | 2 --
drivers/usb/gadget/function/f_ncm.c | 1 -
drivers/usb/gadget/function/f_phonet.c | 1 -
drivers/usb/gadget/function/f_rndis.c | 1 -
drivers/usb/gadget/function/f_subset.c | 1 -
drivers/usb/gadget/function/f_uac2.c | 1 -
7 files changed, 8 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..5c67d28 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -652,8 +652,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..e257eff 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -820,7 +820,6 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *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..c294fb9 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1012,7 +1012,6 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *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)

--
1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Robert Baldyga

2014-10-10 12:35:23 UTC

Permalink

Hi,

Post by pavi1729 ivap

From 3272817673910c63682e8b91ce0efaef190a399a Mon Sep 17 00:00:00 2001

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.

Double-free corruption is good point in this place, but your patch
doesn't solve this problem properly. If usb_assign_descriptors() returns
without error and one of following calls fails, we need to call
usb_free_all_descriptors() in error handling code.

There should be rather two error labels: one where you goto on errors
before usb_assign_descriptors() call, and another one where you goto on
errors after it. And in second case usb_free_all_descriptors() is
called, because descriptors are already allocated and you need to free them.

I also see that in some drivers usb_assign_descriptors() is the last
call in bind() and then removing usb_free_all_descriptors() from error
handling code makes sense.

Best regards,
Robert Baldyga

Post by pavi1729 ivap
---
drivers/usb/gadget/function/f_eem.c | 1 -
drivers/usb/gadget/function/f_hid.c | 2 --
drivers/usb/gadget/function/f_ncm.c | 1 -
drivers/usb/gadget/function/f_phonet.c | 1 -
drivers/usb/gadget/function/f_rndis.c | 1 -
drivers/usb/gadget/function/f_subset.c | 1 -
drivers/usb/gadget/function/f_uac2.c | 1 -
7 files changed, 8 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;
- 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..5c67d28 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -652,8 +652,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;
- 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
for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
- 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..e257eff 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -820,7 +820,6 @@ rndis_bind(struct usb_configuration *c, struct
usb_function *f)
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;
- 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..c294fb9 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1012,7 +1012,6 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
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-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

pavi1729 ivap

2014-10-10 12:51:04 UTC

Permalink

Rob,
Got your point, will submit a new patch with a new error label.
Also I just found that in f_hid.c f_rndis.c and f_uac2.c do need the
freeing function.
Will clean it up and send the patch.

Thanks,
Pavitra

Post by Robert Baldyga
Hi,

Post by pavi1729 ivap

From 3272817673910c63682e8b91ce0efaef190a399a Mon Sep 17 00:00:00 2001

Double-free corruption is good point in this place, but your patch
doesn't solve this problem properly. If usb_assign_descriptors() returns
without error and one of following calls fails, we need to call
usb_free_all_descriptors() in error handling code.
There should be rather two error labels: one where you goto on errors
before usb_assign_descriptors() call, and another one where you goto on
errors after it. And in second case usb_free_all_descriptors() is
called, because descriptors are already allocated and you need to free them.
I also see that in some drivers usb_assign_descriptors() is the last
call in bind() and then removing usb_free_all_descriptors() from error
handling code makes sense.
Best regards,
Robert Baldyga

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

pavi1729 ivap

2014-10-10 12:52:07 UTC

Permalink

Oh! precisely what you are saying ..

Post by pavi1729 ivap
Rob,
Got your point, will submit a new patch with a new error label.
Also I just found that in f_hid.c f_rndis.c and f_uac2.c do need the
freeing function.
Will clean it up and send the patch.
Thanks,
Pavitra

Post by Robert Baldyga
Hi,

Post by pavi1729 ivap

From 3272817673910c63682e8b91ce0efaef190a399a Mon Sep 17 00:00:00 2001

Double-free corruption is good point in this place, but your patch
doesn't solve this problem properly. If usb_assign_descriptors() returns
without error and one of following calls fails, we need to call
usb_free_all_descriptors() in error handling code.
There should be rather two error labels: one where you goto on errors
before usb_assign_descriptors() call, and another one where you goto on
errors after it. And in second case usb_free_all_descriptors() is
called, because descriptors are already allocated and you need to free them.
I also see that in some drivers usb_assign_descriptors() is the last
call in bind() and then removing usb_free_all_descriptors() from error
handling code makes sense.
Best regards,
Robert Baldyga

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

pavi1729 ivap

2014-10-10 13:09:38 UTC

Permalink

From: Pavitra <pavitra1729-***@public.gmane.org>
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-***@public.gmane.org>
---
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:
+ 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:
+ 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:
+ 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)

Robert Baldyga

2014-10-13 08:32:00 UTC

Permalink

Hi,

Post by pavi1729 ivap
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.

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.

Post by pavi1729 ivap
---
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;
- 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;

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).

Post by pavi1729 ivap
+ usb_free_all_descriptors(f);
ERROR(f->config->cdev, "hidg_bind FAILED\n");
if (hidg->req != NULL) {
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;
- 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
for (i = 0; i < phonet_rxq_size && fp->out_reqv[i]; i++)
usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
- 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;

Ditto.

Post by pavi1729 ivap
+ usb_free_all_descriptors(f);
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;
- 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;
+

Ditto again.

Post by pavi1729 ivap
+ usb_free_all_descriptors(fn);
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-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

pavi1729 Sid

2014-10-13 09:02:04 UTC

Permalink

Post by Robert Baldyga
Hi,

You should use your full real name in sign-off (see
Documentation/SubmittingPatches).

will do that.

Post by Robert Baldyga
I see there are no maintainers in your CC list. You should use
scripts/get_maintainer.pl on your patch.

Will do that.

Post by Robert Baldyga

Agreed.

Post by Robert Baldyga

Ditto.

Agreed.

Post by Robert Baldyga

Ditto again.

Agreed.

Post by Robert Baldyga

Thanks,
Robert Baldyga

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html