1. 12 Aug, 2019 1 commit
    • Benjamin Herrenschmidt's avatar
      usb: gadget: mass_storage: Fix races between fsg_disable and fsg_set_alt · 4a56a478
      Benjamin Herrenschmidt authored
      If fsg_disable() and fsg_set_alt() are called too closely to each
      other (for example due to a quick reset/reconnect), what can happen
      is that fsg_set_alt sets common->new_fsg from an interrupt while
      handle_exception is trying to process the config change caused by
      		sets state back to FSG_STATE_NORMAL
      		hasn't yet called do_set_interface()
      		or is inside it.
       ---> interrupt
      		sets common->new_fsg
      		queues a new FSG_STATE_CONFIG_CHANGE
      Now, the first handle_exception can "see" the updated
      new_fsg, treats it as if it was a fsg_set_alt() response,
      call usb_composite_setup_continue() etc...
      But then, the thread sees the second FSG_STATE_CONFIG_CHANGE,
      and goes back down the same path, wipes and reattaches a now
      active fsg, and .. calls usb_composite_setup_continue() which
      at this point is wrong.
      Not only we get a backtrace, but I suspect the second set_interface
      wrecks some state causing the host to get upset in my case.
      This fixes it by replacing "new_fsg" by a "state argument" (same
      principle) which is set in the same lock section as the state
      update, and retrieved similarly.
      That way, there is never any discrepancy between the dequeued
      state and the observed value of it. We keep the ability to have
      the latest reconfig operation take precedence, but we guarantee
      that once "dequeued" the argument (new_fsg) will not be clobbered
      by any new event.
      Signed-off-by: default avatarBenjamin Herrenschmidt <benh@kernel.crashing.org>
      Acked-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  2. 04 Jul, 2019 1 commit
  3. 20 Jun, 2019 1 commit
  4. 18 Jun, 2019 1 commit
    • EJ Hsu's avatar
      usb: gadget: storage: Remove warning message · e70b3f5d
      EJ Hsu authored
      This change is to fix below warning message in following scenario:
      usb_composite_setup_continue: Unexpected call
      When system tried to enter suspend, the fsg_disable() will be called to
      disable fsg driver and send a signal to fsg_main_thread. However, at
      this point, the fsg_main_thread has already been frozen and can not
      respond to this signal. So, this signal will be pended until
      fsg_main_thread wakes up.
      Once system resumes from suspend, fsg_main_thread will detect a signal
      pended and do some corresponding action (in handle_exception()). Then,
      host will send some setup requests (get descriptor, set configuration...)
      to UDC driver trying to enumerate this device. During the handling of "set
      configuration" request, it will try to sync up with fsg_main_thread by
      sending a signal (which is the same as the signal sent by fsg_disable)
      to it. In a similar manner, once the fsg_main_thread receives this
      signal, it will call handle_exception() to handle the request.
      However, if the fsg_main_thread wakes up from suspend a little late and
      "set configuration" request from Host arrives a little earlier,
      fsg_main_thread might come across the request from "set configuration"
      when it handles the signal from fsg_disable(). In this case, it will
      handle this request as well. So, when fsg_main_thread tries to handle
      the signal sent from "set configuration" later, there will nothing left
      to do and warning message "Unexpected call" is printed.
      Acked-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarEJ Hsu <ejh@nvidia.com>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  5. 17 Oct, 2018 1 commit
  6. 11 Sep, 2018 2 commits
  7. 26 Jul, 2018 2 commits
  8. 04 Nov, 2017 1 commit
  9. 19 Oct, 2017 1 commit
  10. 22 Sep, 2017 1 commit
    • Alan Stern's avatar
      USB: g_mass_storage: Fix deadlock when driver is unbound · 1fbbb78f
      Alan Stern authored
      As a holdover from the old g_file_storage gadget, the g_mass_storage
      legacy gadget driver attempts to unregister itself when its main
      operating thread terminates (if it hasn't been unregistered already).
      This is not strictly necessary; it was never more than an attempt to
      have the gadget fail cleanly if something went wrong and the main
      thread was killed.
      However, now that the UDC core manages gadget drivers independently of
      UDC drivers, this scheme doesn't work any more.  A simple test:
      	modprobe dummy-hcd
      	modprobe g-mass-storage file=...
      	rmmod dummy-hcd
      ends up in a deadlock with the following backtrace:
       sysrq: SysRq : Show Blocked State
         task                PC stack   pid father
       file-storage    D    0  1130      2 0x00000000
       Call Trace:
        ? _raw_spin_unlock_irqrestore+0x12/0x14
        usb_gadget_unregister_driver+0x29/0x9b [udc_core]
        usb_composite_unregister+0x10/0x12 [libcomposite]
        msg_cleanup+0x1d/0x20 [g_mass_storage]
        msg_thread_exits+0xd/0xdd7 [g_mass_storage]
        fsg_main_thread+0x1395/0x13d6 [usb_f_mass_storage]
        ? __schedule+0x573/0x58c
        ? do_set_interface+0x25c/0x25c [usb_f_mass_storage]
        ? init_completion+0x1e/0x1e
       rmmod           D    0  1155    683 0x00000000
       Call Trace:
        ? __schedule+0x573/0x58c
        ? usleep_range+0x81/0x81
        ? wake_up_q+0x3f/0x3f
        fsg_common_put+0x34/0x81 [usb_f_mass_storage]
        fsg_free_inst+0x13/0x1e [usb_f_mass_storage]
        usb_put_function_instance+0x1a/0x25 [libcomposite]
        msg_unbind+0x2a/0x42 [g_mass_storage]
        __composite_unbind+0x4a/0x6f [libcomposite]
        composite_unbind+0x12/0x14 [libcomposite]
        usb_gadget_remove_driver+0x4f/0x77 [udc_core]
        usb_del_gadget_udc+0x52/0xcc [udc_core]
        dummy_udc_remove+0x27/0x2c [dummy_hcd]
        ? selinux_capable+0x22/0x27
        cleanup+0x20/0x817 [dummy_hcd]
        ? ____fput+0xd/0xf
        ? task_work_run+0x55/0x62
        ? prepare_exit_to_usermode+0x65/0x75
      What happens is that removing the dummy-hcd driver causes the UDC core
      to unbind the gadget driver, which it does while holding the udc_lock
      mutex.  The unbind routine in g_mass_storage tells the main thread to
      exit and waits for it to terminate.
      But as mentioned above, when the main thread exits it tries to
      unregister the mass-storage function driver.  Via the composite
      framework this ends up calling usb_gadget_unregister_driver(), which
      tries to acquire the udc_lock mutex.  The result is deadlock.
      The simplest way to fix the problem is not to be so clever: The main
      thread doesn't have to unregister the function driver.  The side
      effects won't be so terrible; if the gadget is still attached to a USB
      host when the main thread is killed, it will appear to the host as
      though the gadget's firmware has crashed -- a reasonably accurate
      interpretation, and an all-too-common occurrence for USB mass-storage
      In fact, the code to unregister the driver when the main thread exits
      is specific to g-mass-storage; it is not used when f-mass-storage is
      included as a function in a larger composite device.  Therefore the
      entire mechanism responsible for this (the fsg_operations structure
      with its ->thread_exits method, the fsg_common_set_ops() routine, and
      the msg_thread_exits() callback routine) can all be eliminated.  Even
      the msg_registered bitflag can be removed, because now the driver is
      unregistered in only one place rather than in two places.
      Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      CC: <stable@vger.kernel.org>
      Acked-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
      Acked-by: default avatarMichal Nazarewicz <mina86@mina86.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
  11. 04 Sep, 2017 1 commit
  12. 17 Jul, 2017 1 commit
  13. 02 Jun, 2017 1 commit
    • Thinh Nguyen's avatar
      usb: gadget: f_mass_storage: Serialize wake and sleep execution · dc9217b6
      Thinh Nguyen authored
      f_mass_storage has a memorry barrier issue with the sleep and wake
      functions that can cause a deadlock. This results in intermittent hangs
      during MSC file transfer. The host will reset the device after receiving
      no response to resume the transfer. This issue is seen when dwc3 is
      processing 2 transfer-in-progress events at the same time, invoking
      completion handlers for CSW and CBW. Also this issue occurs depending on
      the system timing and latency.
      To increase the chance to hit this issue, you can force dwc3 driver to
      wait and process those 2 events at once by adding a small delay (~100us)
      in dwc3_check_event_buf() whenever the request is for CSW and read the
      event count again. Avoid debugging with printk and ftrace as extra
      delays and memory barrier will mask this issue.
      Scenario which can lead to failure:
      1) The main thread sleeps and waits for the next command in
      2) bulk_in_complete() wakes up main thread for CSW.
      3) bulk_out_complete() tries to wake up the running main thread for CBW.
      4) thread_wakeup_needed is not loaded with correct value in
      5) Main thread goes to sleep again.
      The pattern is shown below. Note the 2 critical variables.
       * common->thread_wakeup_needed
       * bh->state
      	CPU 0 (sleep_thread)		CPU 1 (wakeup_thread)
      	==============================  ===============================
      					bh->state = BH_STATE_FULL;
      	thread_wakeup_needed = 0;	thread_wakeup_needed = 1;
      	if (bh->state != BH_STATE_FULL)
      		sleep again ...
      As pointed out by Alan Stern, this is an R-pattern issue. The issue can
      be seen when there are two wakeups in quick succession. The
      thread_wakeup_needed can be overwritten in sleep_thread, and the read of
      the bh->state maybe reordered before the write to thread_wakeup_needed.
      This patch applies full memory barrier smp_mb() in both sleep_thread()
      and wakeup_thread() to ensure the order which the thread_wakeup_needed
      and bh->state are written and loaded.
      However, a better solution in the future would be to use wait_queue
      method that takes care of managing memory barrier between waker and
      Cc: <stable@vger.kernel.org>
      Acked-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarThinh Nguyen <thinhn@synopsys.com>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  14. 16 May, 2017 2 commits
    • Alan Stern's avatar
      USB: f_mass_storage: improve memory barriers and synchronization · 225785ae
      Alan Stern authored
      This patch reworks the way f_mass_storage.c handles memory barriers
      and synchronization:
      	The driver now uses a wait_queue instead of doing its own
      	task-state manipulations (even though only one task will ever
      	use the wait_queue).
      	The thread_wakeup_needed variable is removed.  It was only a
      	source of trouble; although it was what the driver tested to
      	see whether it should wake up, what we really wanted to see
      	was whether a USB transfer had completed.
      	All the explicit memory barriers scattered throughout the
      	driver are replaced by a few calls to smp_load_acquire() and
      	The inreq_busy and outreq_busy fields are removed.  In their
      	place, the driver keeps track of the current I/O direction by
      	splitting BUF_STATE_BUSY into two states: BUF_STATE_SENDING
      	The buffer states are no longer protected by a lock.  Mutual
      	exclusion isn't needed; the state is changed only by the
      	driver's main thread when it owns the buffer, and only by the
      	request completion routine when the gadget core owns the buffer.
      	The do_write() and throw_away_data() routines were reorganized
      	to make efficient use of the new sleeping mechanism.  This
      	resulted in the removal of one indentation level in those
      	routines, making the patch appear to be more more complicated
      	than it really is.
      	In a few places, the driver allowed itself to be frozen although
      	it really shouldn't have (in the middle of executing a SCSI
      	command).  Those places have been fixed.
      	The logic in the exception handler for aborting transfers and
      	waiting for them to stop has been simplified.
      Tested-by: default avatarThinh Nguyen <thinhn@synopsys.com>
      Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
    • Alan Stern's avatar
      USB: f_mass_storage: improve async notification handling · 78db441d
      Alan Stern authored
      This patch makes several adjustments to the way f_mass_storage.c
      handles its internal state and asynchronous notifications (AKA
      	A number of states weren't being used for anything.
      	They are removed.
      	The FSG_STATE_IDLE state was renamed to FSG_STATE_NORMAL,
      	because it now applies whenever the gadget is operating
      	normally, not just when the gadget is idle.
      	The FSG_STATE_RESET state was renamed to
      	FSG_STATE_PROTOCOL_RESET, indicating that it represents a
      	Bulk-Only Transport protocol reset and not a general USB
      	When a signal arrives, it's silly for the signal handler to
      	send itself another signal!  Now it takes care of everything
      Along with an assortment of other minor changes in the same category.
      Tested-by: default avatarThinh Nguyen <thinhn@synopsys.com>
      Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  15. 02 Mar, 2017 1 commit
  16. 25 Aug, 2016 1 commit
    • Philipp Gesang's avatar
      usb: gadget: Add per-lun inquiry string · 6ac47090
      Philipp Gesang authored
      Introduce an attribute "inquiry_string" to the lun.
      In some environments, e. g. BIOS boot menus, the inquiry string
      is the only information about devices presented to the user. The
      default string depends on the "cdrom" bit of the first lun as
      well as the kernel version and allows no further customization.
      So without access to the client it is not obvious which gadget is
      active at a given point and what any of the available luns might
      If "inquiry_string" is ignored or set to the empty string, the
      old behavior is preserved.
      Signed-off-by: default avatarPhilipp Gesang <philipp.gesang@intra2net.com>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  17. 20 Jun, 2016 1 commit
  18. 19 Apr, 2016 1 commit
    • Michal Nazarewicz's avatar
      usb: f_mass_storage: test whether thread is running before starting another · f78bbcae
      Michal Nazarewicz authored
      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: default avatarAlan Stern <stern@rowland.harvard.edu>
      Signed-off-by: default avatarMichal Nazarewicz <mina86@mina86.com>
      Tested-by: default avatarIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
      Cc: Alan Stern <stern@rowland.harvard.edu>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
  19. 06 Mar, 2016 1 commit
  20. 04 Mar, 2016 2 commits
  21. 07 Nov, 2015 1 commit
  22. 14 Oct, 2015 1 commit
  23. 27 Sep, 2015 2 commits
  24. 06 Aug, 2015 2 commits
  25. 31 Jul, 2015 2 commits
    • Krzysztof Opasiak's avatar
      usb: gadget: mass_storage: Use static array for luns · dd02ea5a
      Krzysztof Opasiak authored
      This patch replace dynamicly allocated luns array with static one.
      This simplifies the code of mass storage function and modules.
      Signed-off-by: default avatarKrzysztof Opasiak <k.opasiak@samsung.com>
      Acked-by: default avatarMichal Nazarewicz <mina86@mina86.com>
    • Krzysztof Opasiak's avatar
      usb: gadget: mass_storage: Fix freeing luns sysfs implementation · 5542f58c
      Krzysztof Opasiak authored
      Use device_is_registered() instad of sysfs flag to determine if
      we should free sysfs representation of particular LUN.
      sysfs flag in fsg common determines if luns attributes should be
      exposed using sysfs. This flag is used when creating and freeing
      luns. Unfortunately there is no guarantee that this flag will not
      be changed between creation and removal of particular LUN. Especially
      because of lun.0 which is created during allocating instance of
      function. This may lead to resource leak or NULL pointer dereference:
      [   62.539925] Unable to handle kernel NULL pointer dereference at virtual address 00000044
      [   62.548014] pgd = ec994000
      [   62.550679] [00000044] *pgd=6d7be831, *pte=00000000, *ppte=00000000
      [   62.556933] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
      [   62.562310] Modules linked in: g_mass_storage(+)
      [   62.566916] CPU: 2 PID: 613 Comm: insmod Not tainted 4.2.0-rc4-00077-ge29ee91-dirty #125
      [   62.574984] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
      [   62.581061] task: eca56e80 ti: eca76000 task.ti: eca76000
      [   62.586450] PC is at kernfs_find_ns+0x8/0xe8
      [   62.590698] LR is at kernfs_find_and_get_ns+0x30/0x48
      [   62.595732] pc : [<c01277c0>]    lr : [<c0127b88>]    psr: 40010053
      [   62.595732] sp : eca77c40  ip : eca77c38  fp : 000008c1
      [   62.607187] r10: 00000001  r9 : c0082f38  r8 : ed41ce40
      [   62.612395] r7 : c05c1484  r6 : 00000000  r5 : 00000000  r4 : c0814488
      [   62.618904] r3 : 00000000  r2 : 00000000  r1 : c05c1484  r0 : 00000000
      [   62.625417] Flags: nZcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment user
      [   62.632620] Control: 10c5387d  Table: 6c99404a  DAC: 00000015
      [   62.638348] Process insmod (pid: 613, stack limit = 0xeca76210)
      [   62.644251] Stack: (0xeca77c40 to 0xeca78000)
      [   62.648594] 7c40: c0814488 00000000 00000000 c05c1484 ed41ce40 c0127b88 00000000 c0824888
      [   62.656753] 7c60: ed41d038 ed41d030 ed41d000 c012af4c 00000000 c0824858 ed41d038 c02e3314
      [   62.664912] 7c80: ed41d030 00000000 ed41ce04 c02d9e8c c070eda8 eca77cb4 000008c1 c058317c
      [   62.673071] 7ca0: 000008c1 ed41d030 ed41ce00 ed41ce04 ed41d000 c02da044 ed41cf48 c0375870
      [   62.681230] 7cc0: ed9d3c04 ed9d3c00 ed52df80 bf000940 fffffff0 c03758f4 c03758c0 00000000
      [   62.689389] 7ce0: bf000564 c03614e0 ed9d3c04 bf000194 c0082f38 00000001 00000000 c0000100
      [   62.697548] 7d00: c0814488 c0814488 c086b1dc c05893a8 00000000 ed7e8320 00000000 c0128b88
      [   62.705707] 7d20: ed8a6b40 00000000 00000000 ed410500 ed8a6b40 c0594818 ed7e8320 00000000
      [   62.713867] 7d40: 00000000 c0129f20 00000000 c082c444 ed8a6b40 c012a684 00001000 00000000
      [   62.722026] 7d60: c0594818 c082c444 00000000 00000000 ed52df80 ed52df80 00000000 00000000
      [   62.730185] 7d80: 00000000 00000000 00000001 00000002 ed8e9b70 ed52df80 bf0006d0 00000000
      [   62.738345] 7da0: ed8e9b70 ed410500 ed618340 c036129c ed8c1c00 bf0006d0 c080b158 ed8c1c00
      [   62.746504] 7dc0: bf0006d0 c080b158 ed8c1c08 ed410500 c0082f38 ed618340 000008c1 c03640ac
      [   62.754663] 7de0: 00000000 bf0006d0 c082c8dc c080b158 c080b158 c03642d4 00000000 bf003000
      [   62.762822] 7e00: 00000000 c0009784 00000000 00000001 00000000 c05849b0 00000002 ee7ab780
      [   62.770981] 7e20: 00000002 ed4105c0 0000c53e 000000d0 c0808600 eca77e5c 00000004 00000000
      [   62.779140] 7e40: bf000000 c0095680 c08075a0 ee001f00 ed4105c0 c00cadc0 ed52df80 bf000780
      [   62.787300] 7e60: ed4105c0 bf000780 00000001 bf0007c8 c0082f38 ed618340 000008c1 c0083e24
      [   62.795459] 7e80: 00000001 bf000780 00000001 eca77f58 00000001 bf000780 00000001 c00857f4
      [   62.803618] 7ea0: bf00078c 00007fff 00000000 c00835b4 eca77f58 00000000 c0082fac eca77f58
      [   62.811777] 7ec0: f05038c0 0003b008 bf000904 00000000 00000000 bf00078c 6e72656b 00006c65
      [   62.819936] 7ee0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
      [   62.828095] 7f00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
      [   62.836255] 7f20: 00000000 00000000 00000000 00000000 00000000 00000000 00000003 0003b008
      [   62.844414] 7f40: 0000017b c000f5c8 eca76000 00000000 0003b008 c0085df8 f04ef000 0001b8a9
      [   62.852573] 7f60: f0503258 f05030c2 f0509fe8 00000968 00000dc8 00000000 00000000 00000000
      [   62.860732] 7f80: 00000029 0000002a 00000011 00000000 0000000a 00000000 33f6eb00 0003b008
      [   62.868892] 7fa0: bef01cac c000f400 33f6eb00 0003b008 00000003 0003b008 00000000 00000003
      [   62.877051] 7fc0: 33f6eb00 0003b008 bef01cac 0000017b 00000000 0003b008 0000000b 0003b008
      [   62.885210] 7fe0: bef01ae0 bef01ad0 0001dc23 b6e8c162 800b0070 00000003 c0c0c0c0 c0c0c0c0
      [   62.893380] [<c01277c0>] (kernfs_find_ns) from [<c0824888>] (pm_qos_latency_tolerance_attr_group+0x0/0x10)
      [   62.903005] Code: e28dd00c e8bd80f0 e92d41f0 e2923000 (e1d0e4b4)
      [   62.909115] ---[ end trace 02fb4373ef095c7b ]---
      Acked-by: default avatarMichal Nazarewicz <mina86@mina86.com>
      Signed-off-by: default avatarKrzysztof Opasiak <k.opasiak@samsung.com>
      Signed-off-by: default avatarFelipe Balbi <balbi@ti.com>
  26. 30 Jul, 2015 2 commits
  27. 29 Jul, 2015 3 commits
  28. 06 Jul, 2015 1 commit
  29. 23 Jun, 2015 1 commit
  30. 18 Mar, 2015 1 commit