releases/4.4.92/usb-g_mass_storage-fix-deadlock-when-driver-is-unbound.patch

   1 From 1fbbb78f25d1291274f320462bf6908906f538db Mon Sep 17 00:00:00 2001
   2 From: Alan Stern <stern@rowland.harvard.edu>
   3 Date: Thu, 21 Sep 2017 13:22:00 -0400
   4 Subject: USB: g_mass_storage: Fix deadlock when driver is unbound
   5
   6 From: Alan Stern <stern@rowland.harvard.edu>
   7
   8 commit 1fbbb78f25d1291274f320462bf6908906f538db upstream.
   9
  10 As a holdover from the old g_file_storage gadget, the g_mass_storage
  11 legacy gadget driver attempts to unregister itself when its main
  12 operating thread terminates (if it hasn't been unregistered already).
  13 This is not strictly necessary; it was never more than an attempt to
  14 have the gadget fail cleanly if something went wrong and the main
  15 thread was killed.
  16
  17 However, now that the UDC core manages gadget drivers independently of
  18 UDC drivers, this scheme doesn't work any more.  A simple test:
  19
  20         modprobe dummy-hcd
  21         modprobe g-mass-storage file=...
  22         rmmod dummy-hcd
  23
  24 ends up in a deadlock with the following backtrace:
  25
  26  sysrq: SysRq : Show Blocked State
  27    task                PC stack   pid father
  28  file-storage    D    0  1130      2 0x00000000
  29  Call Trace:
  30   __schedule+0x53e/0x58c
  31   schedule+0x6e/0x77
  32   schedule_preempt_disabled+0xd/0xf
  33   __mutex_lock.isra.1+0x129/0x224
  34   ? _raw_spin_unlock_irqrestore+0x12/0x14
  35   __mutex_lock_slowpath+0x12/0x14
  36   mutex_lock+0x28/0x2b
  37   usb_gadget_unregister_driver+0x29/0x9b [udc_core]
  38   usb_composite_unregister+0x10/0x12 [libcomposite]
  39   msg_cleanup+0x1d/0x20 [g_mass_storage]
  40   msg_thread_exits+0xd/0xdd7 [g_mass_storage]
  41   fsg_main_thread+0x1395/0x13d6 [usb_f_mass_storage]
  42   ? __schedule+0x573/0x58c
  43   kthread+0xd9/0xdb
  44   ? do_set_interface+0x25c/0x25c [usb_f_mass_storage]
  45   ? init_completion+0x1e/0x1e
  46   ret_from_fork+0x19/0x24
  47  rmmod           D    0  1155    683 0x00000000
  48  Call Trace:
  49   __schedule+0x53e/0x58c
  50   schedule+0x6e/0x77
  51   schedule_timeout+0x26/0xbc
  52   ? __schedule+0x573/0x58c
  53   do_wait_for_common+0xb3/0x128
  54   ? usleep_range+0x81/0x81
  55   ? wake_up_q+0x3f/0x3f
  56   wait_for_common+0x2e/0x45
  57   wait_for_completion+0x17/0x19
  58   fsg_common_put+0x34/0x81 [usb_f_mass_storage]
  59   fsg_free_inst+0x13/0x1e [usb_f_mass_storage]
  60   usb_put_function_instance+0x1a/0x25 [libcomposite]
  61   msg_unbind+0x2a/0x42 [g_mass_storage]
  62   __composite_unbind+0x4a/0x6f [libcomposite]
  63   composite_unbind+0x12/0x14 [libcomposite]
  64   usb_gadget_remove_driver+0x4f/0x77 [udc_core]
  65   usb_del_gadget_udc+0x52/0xcc [udc_core]
  66   dummy_udc_remove+0x27/0x2c [dummy_hcd]
  67   platform_drv_remove+0x1d/0x31
  68   device_release_driver_internal+0xe9/0x16d
  69   device_release_driver+0x11/0x13
  70   bus_remove_device+0xd2/0xe2
  71   device_del+0x19f/0x221
  72   ? selinux_capable+0x22/0x27
  73   platform_device_del+0x21/0x63
  74   platform_device_unregister+0x10/0x1a
  75   cleanup+0x20/0x817 [dummy_hcd]
  76   SyS_delete_module+0x10c/0x197
  77   ? ____fput+0xd/0xf
  78   ? task_work_run+0x55/0x62
  79   ? prepare_exit_to_usermode+0x65/0x75
  80   do_fast_syscall_32+0x86/0xc3
  81   entry_SYSENTER_32+0x4e/0x7c
  82
  83 What happens is that removing the dummy-hcd driver causes the UDC core
  84 to unbind the gadget driver, which it does while holding the udc_lock
  85 mutex.  The unbind routine in g_mass_storage tells the main thread to
  86 exit and waits for it to terminate.
  87
  88 But as mentioned above, when the main thread exits it tries to
  89 unregister the mass-storage function driver.  Via the composite
  90 framework this ends up calling usb_gadget_unregister_driver(), which
  91 tries to acquire the udc_lock mutex.  The result is deadlock.
  92
  93 The simplest way to fix the problem is not to be so clever: The main
  94 thread doesn't have to unregister the function driver.  The side
  95 effects won't be so terrible; if the gadget is still attached to a USB
  96 host when the main thread is killed, it will appear to the host as
  97 though the gadget's firmware has crashed -- a reasonably accurate
  98 interpretation, and an all-too-common occurrence for USB mass-storage
  99 devices.
 100
 101 In fact, the code to unregister the driver when the main thread exits
 102 is specific to g-mass-storage; it is not used when f-mass-storage is
 103 included as a function in a larger composite device.  Therefore the
 104 entire mechanism responsible for this (the fsg_operations structure
 105 with its ->thread_exits method, the fsg_common_set_ops() routine, and
 106 the msg_thread_exits() callback routine) can all be eliminated.  Even
 107 the msg_registered bitflag can be removed, because now the driver is
 108 unregistered in only one place rather than in two places.
 109
 110 Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
 111 Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
 112 Acked-by: Michal Nazarewicz <mina86@mina86.com>
 113 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 114
 115 ---
 116  drivers/usb/gadget/function/f_mass_storage.c |   29 +++++++--------------------
 117  drivers/usb/gadget/function/f_mass_storage.h |   14 -------------
 118  drivers/usb/gadget/legacy/mass_storage.c     |   26 ++----------------------
 119  3 files changed, 11 insertions(+), 58 deletions(-)
 120
 121 --- a/drivers/usb/gadget/function/f_mass_storage.c
 122 +++ b/drivers/usb/gadget/function/f_mass_storage.c
 123 @@ -306,8 +306,6 @@ struct fsg_common {
 124         struct completion       thread_notifier;
 125         struct task_struct      *thread_task;
 126
 127 -       /* Callback functions. */
 128 -       const struct fsg_operations     *ops;
 129         /* Gadget's private data. */
 130         void                    *private_data;
 131
 132 @@ -2504,6 +2502,7 @@ static void handle_exception(struct fsg_
 133  static int fsg_main_thread(void *common_)
 134  {
 135         struct fsg_common       *common = common_;
 136 +       int                     i;
 137
 138         /*
 139          * Allow the thread to be killed by a signal, but set the signal mask
 140 @@ -2565,21 +2564,16 @@ static int fsg_main_thread(void *common_
 141         common->thread_task = NULL;
 142         spin_unlock_irq(&common->lock);
 143
 144 -       if (!common->ops || !common->ops->thread_exits
 145 -        || common->ops->thread_exits(common) < 0) {
 146 -               int i;
 147 -
 148 -               down_write(&common->filesem);
 149 -               for (i = 0; i < ARRAY_SIZE(common->luns); --i) {
 150 -                       struct fsg_lun *curlun = common->luns[i];
 151 -                       if (!curlun || !fsg_lun_is_open(curlun))
 152 -                               continue;
 153 +       /* Eject media from all LUNs */
 154
 155 +       down_write(&common->filesem);
 156 +       for (i = 0; i < ARRAY_SIZE(common->luns); i++) {
 157 +               struct fsg_lun *curlun = common->luns[i];
 158 +
 159 +               if (curlun && fsg_lun_is_open(curlun))
 160                         fsg_lun_close(curlun);
 161 -                       curlun->unit_attention_data = SS_MEDIUM_NOT_PRESENT;
 162 -               }
 163 -               up_write(&common->filesem);
 164         }
 165 +       up_write(&common->filesem);
 166
 167         /* Let fsg_unbind() know the thread has exited */
 168         complete_and_exit(&common->thread_notifier, 0);
 169 @@ -2785,13 +2779,6 @@ void fsg_common_remove_luns(struct fsg_c
 170  }
 171  EXPORT_SYMBOL_GPL(fsg_common_remove_luns);
 172
 173 -void fsg_common_set_ops(struct fsg_common *common,
 174 -                       const struct fsg_operations *ops)
 175 -{
 176 -       common->ops = ops;
 177 -}
 178 -EXPORT_SYMBOL_GPL(fsg_common_set_ops);
 179 -
 180  void fsg_common_free_buffers(struct fsg_common *common)
 181  {
 182         _fsg_common_free_buffers(common->buffhds, common->fsg_num_buffers);
 183 --- a/drivers/usb/gadget/function/f_mass_storage.h
 184 +++ b/drivers/usb/gadget/function/f_mass_storage.h
 185 @@ -60,17 +60,6 @@ struct fsg_module_parameters {
 186  struct fsg_common;
 187
 188  /* FSF callback functions */
 189 -struct fsg_operations {
 190 -       /*
 191 -        * Callback function to call when thread exits.  If no
 192 -        * callback is set or it returns value lower then zero MSF
 193 -        * will force eject all LUNs it operates on (including those
 194 -        * marked as non-removable or with prevent_medium_removal flag
 195 -        * set).
 196 -        */
 197 -       int (*thread_exits)(struct fsg_common *common);
 198 -};
 199 -
 200  struct fsg_lun_opts {
 201         struct config_group group;
 202         struct fsg_lun *lun;
 203 @@ -141,9 +130,6 @@ void fsg_common_remove_lun(struct fsg_lu
 204
 205  void fsg_common_remove_luns(struct fsg_common *common);
 206
 207 -void fsg_common_set_ops(struct fsg_common *common,
 208 -                       const struct fsg_operations *ops);
 209 -
 210  int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
 211                           unsigned int id, const char *name,
 212                           const char **name_pfx);
 213 --- a/drivers/usb/gadget/legacy/mass_storage.c
 214 +++ b/drivers/usb/gadget/legacy/mass_storage.c
 215 @@ -107,15 +107,6 @@ static unsigned int fsg_num_buffers = CO
 216
 217  FSG_MODULE_PARAMETERS(/* no prefix */, mod_data);
 218
 219 -static unsigned long msg_registered;
 220 -static void msg_cleanup(void);
 221 -
 222 -static int msg_thread_exits(struct fsg_common *common)
 223 -{
 224 -       msg_cleanup();
 225 -       return 0;
 226 -}
 227 -
 228  static int msg_do_config(struct usb_configuration *c)
 229  {
 230         struct fsg_opts *opts;
 231 @@ -154,9 +145,6 @@ static struct usb_configuration msg_conf
 232
 233  static int msg_bind(struct usb_composite_dev *cdev)
 234  {
 235 -       static const struct fsg_operations ops = {
 236 -               .thread_exits = msg_thread_exits,
 237 -       };
 238         struct fsg_opts *opts;
 239         struct fsg_config config;
 240         int status;
 241 @@ -173,8 +161,6 @@ static int msg_bind(struct usb_composite
 242         if (status)
 243                 goto fail;
 244
 245 -       fsg_common_set_ops(opts->common, &ops);
 246 -
 247         status = fsg_common_set_cdev(opts->common, cdev, config.can_stall);
 248         if (status)
 249                 goto fail_set_cdev;
 250 @@ -256,18 +242,12 @@ MODULE_LICENSE("GPL");
 251
 252  static int __init msg_init(void)
 253  {
 254 -       int ret;
 255 -
 256 -       ret = usb_composite_probe(&msg_driver);
 257 -       set_bit(0, &msg_registered);
 258 -
 259 -       return ret;
 260 +       return usb_composite_probe(&msg_driver);
 261  }
 262  module_init(msg_init);
 263
 264 -static void msg_cleanup(void)
 265 +static void __exit msg_cleanup(void)
 266  {
 267 -       if (test_and_clear_bit(0, &msg_registered))
 268 -               usb_composite_unregister(&msg_driver);
 269 +       usb_composite_unregister(&msg_driver);
 270  }
 271  module_exit(msg_cleanup);