linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lianwei Wang <lianwei.wang@gmail.com>
To: balbi@kernel.org
Cc: linux-usb@vger.kernel.org, gregkh@linuxfoundation.org,
	lianwei.wang@gmail.com
Subject: [PATCH] usb: gadget: avoid using gadget after freed
Date: Fri, 14 Jun 2019 00:02:43 -0700	[thread overview]
Message-ID: <20190614070243.31565-1-lianwei.wang@gmail.com> (raw)

The udc and gadget device will be deleted when udc device is
disconnected and the related function will be unbind with it.

But if the configfs is not deleted, then the function object
will be kept and the bound status is kept true.

Then after udc device is connected again and a new udc and
gadget objects will be created and passed to bind interface.
But because the bound is still true, the new gadget is not
updated to netdev and a previous freed gadget will be used
in netdev after bind.

To fix this using after freed issue, always set the gadget
object to netdev in bind interface.

Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
---
 drivers/usb/gadget/function/f_ecm.c    | 10 ++++++----
 drivers/usb/gadget/function/f_eem.c    | 10 ++++++----
 drivers/usb/gadget/function/f_ncm.c    | 11 +++++++----
 drivers/usb/gadget/function/f_phonet.c |  2 +-
 drivers/usb/gadget/function/f_rndis.c  |  2 +-
 drivers/usb/gadget/function/f_subset.c | 10 ++++++----
 6 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 6ce044008cf6..ff39b7eafec7 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -695,15 +695,17 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to ecm_opts->bound access
 	 */
+	mutex_lock(&ecm_opts->lock);
+	gether_set_gadget(ecm_opts->net, cdev->gadget);
 	if (!ecm_opts->bound) {
-		mutex_lock(&ecm_opts->lock);
-		gether_set_gadget(ecm_opts->net, cdev->gadget);
 		status = gether_register_netdev(ecm_opts->net);
-		mutex_unlock(&ecm_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&ecm_opts->lock);
 			return status;
+		}
 		ecm_opts->bound = true;
 	}
+	mutex_unlock(&ecm_opts->lock);
 
 	ecm_string_defs[1].s = ecm->ethaddr;
 
diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
index c13befa31110..589862dfe1e7 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -256,15 +256,17 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to eem_opts->bound access
 	 */
+	mutex_lock(&eem_opts->lock);
+	gether_set_gadget(eem_opts->net, cdev->gadget);
 	if (!eem_opts->bound) {
-		mutex_lock(&eem_opts->lock);
-		gether_set_gadget(eem_opts->net, cdev->gadget);
 		status = gether_register_netdev(eem_opts->net);
-		mutex_unlock(&eem_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&eem_opts->lock);
 			return status;
+		}
 		eem_opts->bound = true;
 	}
+	mutex_unlock(&eem_opts->lock);
 
 	us = usb_gstrings_attach(cdev, eem_strings,
 				 ARRAY_SIZE(eem_string_defs));
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 2d6e76e4cffa..951867e230c2 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1409,15 +1409,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to ncm_opts->bound access
 	 */
+	mutex_lock(&ncm_opts->lock);
+	gether_set_gadget(ncm_opts->net, cdev->gadget);
 	if (!ncm_opts->bound) {
-		mutex_lock(&ncm_opts->lock);
-		gether_set_gadget(ncm_opts->net, cdev->gadget);
 		status = gether_register_netdev(ncm_opts->net);
-		mutex_unlock(&ncm_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&ncm_opts->lock);
 			goto fail;
+		}
 		ncm_opts->bound = true;
 	}
+	mutex_unlock(&ncm_opts->lock);
+
 	us = usb_gstrings_attach(cdev, ncm_strings,
 				 ARRAY_SIZE(ncm_string_defs));
 	if (IS_ERR(us)) {
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index 8b72b192c747..e93c5cf95494 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -494,8 +494,8 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to phonet_opts->bound access
 	 */
+	gphonet_set_gadget(phonet_opts->net, gadget);
 	if (!phonet_opts->bound) {
-		gphonet_set_gadget(phonet_opts->net, gadget);
 		status = gphonet_register_netdev(phonet_opts->net);
 		if (status)
 			return status;
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index d48df36622b7..f7e59b482d55 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -698,8 +698,8 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to rndis_opts->bound access
 	 */
+	gether_set_gadget(rndis_opts->net, cdev->gadget);
 	if (!rndis_opts->bound) {
-		gether_set_gadget(rndis_opts->net, cdev->gadget);
 		status = gether_register_netdev(rndis_opts->net);
 		if (status)
 			goto fail;
diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index 4d945254905d..878c0eb9efbd 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -308,15 +308,17 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to gether_opts->bound access
 	 */
+	mutex_lock(&gether_opts->lock);
+	gether_set_gadget(gether_opts->net, cdev->gadget);
 	if (!gether_opts->bound) {
-		mutex_lock(&gether_opts->lock);
-		gether_set_gadget(gether_opts->net, cdev->gadget);
 		status = gether_register_netdev(gether_opts->net);
-		mutex_unlock(&gether_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&gether_opts->lock);
 			return status;
+		}
 		gether_opts->bound = true;
 	}
+	mutex_unlock(&gether_opts->lock);
 
 	us = usb_gstrings_attach(cdev, geth_strings,
 				 ARRAY_SIZE(geth_string_defs));
-- 
2.17.1


             reply	other threads:[~2019-06-14  7:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-14  7:02 Lianwei Wang [this message]
2019-06-17 12:40 ` [PATCH] usb: gadget: avoid using gadget after freed Felipe Balbi
2019-06-17 20:15   ` Lianwei Wang
2019-06-18  7:21     ` Felipe Balbi
2019-06-19  3:27       ` Lianwei Wang
2019-06-19  6:21         ` Felipe Balbi
2019-06-20  3:52           ` Lianwei Wang
2019-06-20  5:55             ` Felipe Balbi
2019-06-20  6:29               ` Lianwei Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190614070243.31565-1-lianwei.wang@gmail.com \
    --to=lianwei.wang@gmail.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).