[3.19.y-ckt stable] Patch "usb: f_mass_storage: test whether thread is running before starting another" has been added to the 3.19.y-ckt tree

Kamal Mostafa kamal at canonical.com
Wed Jul 6 21:00:10 UTC 2016


This is a note to let you know that I have just added a patch titled

    usb: f_mass_storage: test whether thread is running before starting another

to the linux-3.19.y-queue branch of the 3.19.y-ckt extended stable tree 
which can be found at:

    https://git.launchpad.net/~canonical-kernel/linux/+git/linux-stable-ckt/log/?h=linux-3.19.y-queue

This patch is scheduled to be released in version 3.19.8-ckt23.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.19.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

---8<------------------------------------------------------------

>From 0788846478bf53dcb638447219e467bc88cbcbef Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86 at mina86.com>
Date: Fri, 8 Apr 2016 10:24:11 +0200
Subject: usb: f_mass_storage: test whether thread is running before starting
 another
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit f78bbcae86e676fad9e6c6bb6cd9d9868ba23696 upstream.

When binding the function to usb_configuration, check whether the thread
is running before starting another one.  Without that, when function
instance is added to multiple configurations, fsg_bing starts multiple
threads with all but the latest one being forgotten by the driver.  This
leads to obvious thread leaks, possible lockups when trying to halt the
machine and possible more issues.

This fixes issues with legacy/multi¹ gadget as well as configfs gadgets
when mass_storage function is added to multiple configurations.

This change also simplifies API since the legacy gadgets no longer need
to worry about starting the thread by themselves (which was where bug
in legacy/multi was in the first place).

N.B., this patch doesn’t address adding single mass_storage function
instance to a single configuration twice.  Thankfully, there’s no
legitimate reason for such setup plus, if I’m not mistaken, configfs
gadget doesn’t even allow it to be expressed.

¹ I have no example failure though.  Conclusion that legacy/multi has
  a bug is based purely on me reading the code.

Acked-by: Alan Stern <stern at rowland.harvard.edu>
Signed-off-by: Michal Nazarewicz <mina86 at mina86.com>
Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75 at gmail.com>
Cc: Alan Stern <stern at rowland.harvard.edu>
Signed-off-by: Felipe Balbi <felipe.balbi at linux.intel.com>
[ kamal: backport to 4.2-stable: fsg_bind() decl 'common';
  no change to nokia.c (no fsg_opts) ]
Signed-off-by: Kamal Mostafa <kamal at canonical.com>

squash! 334f47b
---
 drivers/usb/gadget/function/f_mass_storage.c | 37 ++++++++++++----------------
 drivers/usb/gadget/function/f_mass_storage.h |  2 --
 drivers/usb/gadget/legacy/acm_ms.c           |  4 ---
 drivers/usb/gadget/legacy/mass_storage.c     |  4 ---
 drivers/usb/gadget/legacy/multi.c            | 12 ---------
 5 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index eb80e97..74f8f36 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3037,25 +3037,6 @@ void fsg_common_set_inquiry_string(struct fsg_common *common, const char *vn,
 }
 EXPORT_SYMBOL_GPL(fsg_common_set_inquiry_string);

-int fsg_common_run_thread(struct fsg_common *common)
-{
-	common->state = FSG_STATE_IDLE;
-	/* Tell the thread to start working */
-	common->thread_task =
-		kthread_create(fsg_main_thread, common, "file-storage");
-	if (IS_ERR(common->thread_task)) {
-		common->state = FSG_STATE_TERMINATED;
-		return PTR_ERR(common->thread_task);
-	}
-
-	DBG(common, "I/O thread pid: %d\n", task_pid_nr(common->thread_task));
-
-	wake_up_process(common->thread_task);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(fsg_common_run_thread);
-
 static void fsg_common_release(struct kref *ref)
 {
 	struct fsg_common *common = container_of(ref, struct fsg_common, ref);
@@ -3064,6 +3045,7 @@ static void fsg_common_release(struct kref *ref)
 	if (common->state != FSG_STATE_TERMINATED) {
 		raise_exception(common, FSG_STATE_EXIT);
 		wait_for_completion(&common->thread_notifier);
+		common->thread_task = NULL;
 	}

 	if (likely(common->luns)) {
@@ -3097,6 +3079,7 @@ static void fsg_common_release(struct kref *ref)
 static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct fsg_dev		*fsg = fsg_from_func(f);
+	struct fsg_common       *common = fsg->common;
 	struct usb_gadget	*gadget = c->cdev->gadget;
 	int			i;
 	struct usb_ep		*ep;
@@ -3111,9 +3094,21 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
 		if (ret)
 			return ret;
 		fsg_common_set_inquiry_string(fsg->common, NULL, NULL);
-		ret = fsg_common_run_thread(fsg->common);
-		if (ret)
+	}
+
+	if (!common->thread_task) {
+		common->state = FSG_STATE_IDLE;
+		common->thread_task =
+			kthread_create(fsg_main_thread, common, "file-storage");
+		if (IS_ERR(common->thread_task)) {
+			int ret = PTR_ERR(common->thread_task);
+			common->thread_task = NULL;
+			common->state = FSG_STATE_TERMINATED;
 			return ret;
+		}
+		DBG(common, "I/O thread pid: %d\n",
+		    task_pid_nr(common->thread_task));
+		wake_up_process(common->thread_task);
 	}

 	fsg->gadget = gadget;
diff --git a/drivers/usb/gadget/function/f_mass_storage.h b/drivers/usb/gadget/function/f_mass_storage.h
index b4866fc..44f2337 100644
--- a/drivers/usb/gadget/function/f_mass_storage.h
+++ b/drivers/usb/gadget/function/f_mass_storage.h
@@ -157,8 +157,6 @@ int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg);
 void fsg_common_set_inquiry_string(struct fsg_common *common, const char *vn,
 				   const char *pn);

-int fsg_common_run_thread(struct fsg_common *common);
-
 void fsg_config_from_params(struct fsg_config *cfg,
 			    const struct fsg_module_parameters *params,
 			    unsigned int fsg_num_buffers);
diff --git a/drivers/usb/gadget/legacy/acm_ms.c b/drivers/usb/gadget/legacy/acm_ms.c
index c30b7b5..654f11e 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -147,10 +147,6 @@ static int __init acm_ms_do_config(struct usb_configuration *c)
 	if (status < 0)
 		goto put_msg;

-	status = fsg_common_run_thread(opts->common);
-	if (status)
-		goto remove_acm;
-
 	status = usb_add_function(c, f_msg);
 	if (status)
 		goto remove_acm;
diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
index 8e27a8c..ae023d0 100644
--- a/drivers/usb/gadget/legacy/mass_storage.c
+++ b/drivers/usb/gadget/legacy/mass_storage.c
@@ -146,10 +146,6 @@ static int __init msg_do_config(struct usb_configuration *c)
 	if (IS_ERR(f_msg))
 		return PTR_ERR(f_msg);

-	ret = fsg_common_run_thread(opts->common);
-	if (ret)
-		goto put_func;
-
 	ret = usb_add_function(c, f_msg);
 	if (ret)
 		goto put_func;
diff --git a/drivers/usb/gadget/legacy/multi.c b/drivers/usb/gadget/legacy/multi.c
index 39d27bb..f87a8a3 100644
--- a/drivers/usb/gadget/legacy/multi.c
+++ b/drivers/usb/gadget/legacy/multi.c
@@ -151,7 +151,6 @@ static struct usb_function *f_msg_rndis;

 static __init int rndis_do_config(struct usb_configuration *c)
 {
-	struct fsg_opts *fsg_opts;
 	int ret;

 	if (gadget_is_otg(c->cdev->gadget)) {
@@ -183,11 +182,6 @@ static __init int rndis_do_config(struct usb_configuration *c)
 		goto err_fsg;
 	}

-	fsg_opts = fsg_opts_from_func_inst(fi_msg);
-	ret = fsg_common_run_thread(fsg_opts->common);
-	if (ret)
-		goto err_run;
-
 	ret = usb_add_function(c, f_msg_rndis);
 	if (ret)
 		goto err_run;
@@ -239,7 +233,6 @@ static struct usb_function *f_msg_multi;

 static __init int cdc_do_config(struct usb_configuration *c)
 {
-	struct fsg_opts *fsg_opts;
 	int ret;

 	if (gadget_is_otg(c->cdev->gadget)) {
@@ -272,11 +265,6 @@ static __init int cdc_do_config(struct usb_configuration *c)
 		goto err_fsg;
 	}

-	fsg_opts = fsg_opts_from_func_inst(fi_msg);
-	ret = fsg_common_run_thread(fsg_opts->common);
-	if (ret)
-		goto err_run;
-
 	ret = usb_add_function(c, f_msg_multi);
 	if (ret)
 		goto err_run;
--
2.7.4





More information about the kernel-team mailing list