Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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

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@rowland.harvard.edu>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Tested-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: <stable@vger.kernel.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

authored by

Michal Nazarewicz and committed by
Felipe Balbi
f78bbcae 332a5b44

+15 -50
+15 -21
drivers/usb/gadget/function/f_mass_storage.c
··· 2977 2977 } 2978 2978 EXPORT_SYMBOL_GPL(fsg_common_set_inquiry_string); 2979 2979 2980 - int fsg_common_run_thread(struct fsg_common *common) 2981 - { 2982 - common->state = FSG_STATE_IDLE; 2983 - /* Tell the thread to start working */ 2984 - common->thread_task = 2985 - kthread_create(fsg_main_thread, common, "file-storage"); 2986 - if (IS_ERR(common->thread_task)) { 2987 - common->state = FSG_STATE_TERMINATED; 2988 - return PTR_ERR(common->thread_task); 2989 - } 2990 - 2991 - DBG(common, "I/O thread pid: %d\n", task_pid_nr(common->thread_task)); 2992 - 2993 - wake_up_process(common->thread_task); 2994 - 2995 - return 0; 2996 - } 2997 - EXPORT_SYMBOL_GPL(fsg_common_run_thread); 2998 - 2999 2980 static void fsg_common_release(struct kref *ref) 3000 2981 { 3001 2982 struct fsg_common *common = container_of(ref, struct fsg_common, ref); ··· 2986 3005 if (common->state != FSG_STATE_TERMINATED) { 2987 3006 raise_exception(common, FSG_STATE_EXIT); 2988 3007 wait_for_completion(&common->thread_notifier); 3008 + common->thread_task = NULL; 2989 3009 } 2990 3010 2991 3011 for (i = 0; i < ARRAY_SIZE(common->luns); ++i) { ··· 3032 3050 if (ret) 3033 3051 return ret; 3034 3052 fsg_common_set_inquiry_string(fsg->common, NULL, NULL); 3035 - ret = fsg_common_run_thread(fsg->common); 3036 - if (ret) 3053 + } 3054 + 3055 + if (!common->thread_task) { 3056 + common->state = FSG_STATE_IDLE; 3057 + common->thread_task = 3058 + kthread_create(fsg_main_thread, common, "file-storage"); 3059 + if (IS_ERR(common->thread_task)) { 3060 + int ret = PTR_ERR(common->thread_task); 3061 + common->thread_task = NULL; 3062 + common->state = FSG_STATE_TERMINATED; 3037 3063 return ret; 3064 + } 3065 + DBG(common, "I/O thread pid: %d\n", 3066 + task_pid_nr(common->thread_task)); 3067 + wake_up_process(common->thread_task); 3038 3068 } 3039 3069 3040 3070 fsg->gadget = gadget;
-2
drivers/usb/gadget/function/f_mass_storage.h
··· 153 153 void fsg_common_set_inquiry_string(struct fsg_common *common, const char *vn, 154 154 const char *pn); 155 155 156 - int fsg_common_run_thread(struct fsg_common *common); 157 - 158 156 void fsg_config_from_params(struct fsg_config *cfg, 159 157 const struct fsg_module_parameters *params, 160 158 unsigned int fsg_num_buffers);
-4
drivers/usb/gadget/legacy/acm_ms.c
··· 133 133 if (status < 0) 134 134 goto put_msg; 135 135 136 - status = fsg_common_run_thread(opts->common); 137 - if (status) 138 - goto remove_acm; 139 - 140 136 status = usb_add_function(c, f_msg); 141 137 if (status) 142 138 goto remove_acm;
-4
drivers/usb/gadget/legacy/mass_storage.c
··· 132 132 if (IS_ERR(f_msg)) 133 133 return PTR_ERR(f_msg); 134 134 135 - ret = fsg_common_run_thread(opts->common); 136 - if (ret) 137 - goto put_func; 138 - 139 135 ret = usb_add_function(c, f_msg); 140 136 if (ret) 141 137 goto put_func;
-12
drivers/usb/gadget/legacy/multi.c
··· 137 137 138 138 static int rndis_do_config(struct usb_configuration *c) 139 139 { 140 - struct fsg_opts *fsg_opts; 141 140 int ret; 142 141 143 142 if (gadget_is_otg(c->cdev->gadget)) { ··· 167 168 ret = PTR_ERR(f_msg_rndis); 168 169 goto err_fsg; 169 170 } 170 - 171 - fsg_opts = fsg_opts_from_func_inst(fi_msg); 172 - ret = fsg_common_run_thread(fsg_opts->common); 173 - if (ret) 174 - goto err_run; 175 171 176 172 ret = usb_add_function(c, f_msg_rndis); 177 173 if (ret) ··· 219 225 220 226 static int cdc_do_config(struct usb_configuration *c) 221 227 { 222 - struct fsg_opts *fsg_opts; 223 228 int ret; 224 229 225 230 if (gadget_is_otg(c->cdev->gadget)) { ··· 250 257 ret = PTR_ERR(f_msg_multi); 251 258 goto err_fsg; 252 259 } 253 - 254 - fsg_opts = fsg_opts_from_func_inst(fi_msg); 255 - ret = fsg_common_run_thread(fsg_opts->common); 256 - if (ret) 257 - goto err_run; 258 260 259 261 ret = usb_add_function(c, f_msg_multi); 260 262 if (ret)
-7
drivers/usb/gadget/legacy/nokia.c
··· 152 152 struct usb_function *f_ecm; 153 153 struct usb_function *f_obex2 = NULL; 154 154 struct usb_function *f_msg; 155 - struct fsg_opts *fsg_opts; 156 155 int status = 0; 157 156 int obex1_stat = -1; 158 157 int obex2_stat = -1; ··· 220 221 pr_debug("could not bind ecm config %d\n", status); 221 222 goto err_ecm; 222 223 } 223 - 224 - fsg_opts = fsg_opts_from_func_inst(fi_msg); 225 - 226 - status = fsg_common_run_thread(fsg_opts->common); 227 - if (status) 228 - goto err_msg; 229 224 230 225 status = usb_add_function(c, f_msg); 231 226 if (status)