1. 19 May, 2017 3 commits
    • Douglas Anderson's avatar
      mwifiex: Don't release tx_ba_stream_tbl_lock while iterating · e0b636e5
      Douglas Anderson authored
      Despite the macro list_for_each_entry_safe() having the word "safe" in
      the name, it's still not actually safe to release the list spinlock
      while iterating over the list.  The "safe" in the macro name actually
      only means that it's safe to delete the current entry while iterating
      over the list.
      
      Releasing the spinlock while iterating over the list means that someone
      else could come in and adjust the list while we don't have the
      spinlock.  If they do that it can totally mix up our iteration and fully
      corrupt the list.  Later iterating over a corrupted list while holding a
      spinlock and having IRQs off can cause all sorts of hard to debug
      problems.
      
      As evidenced by the other call to
      mwifiex_11n_delete_tx_ba_stream_tbl_entry() in
      mwifiex_11n_delete_all_tx_ba_stream_tbl(), it's actually safe to skip
      the spinlock release.  Let's do that.
      
      No known problems are fixed by this patch, but it could fix all sorts of
      weird problems and it should be very safe.
      Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
      Signed-off-by: default avatarBrian Norris <briannorris@chromium.org>
      Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
      e0b636e5
    • Brian Norris's avatar
      mwifiex: fixup error cases in mwifiex_add_virtual_intf() · 8535107a
      Brian Norris authored
      If we fail to add an interface in mwifiex_add_virtual_intf(), we might
      hit a BUG_ON() in the networking code, because we didn't tear things
      down properly. Among the problems:
      
       (a) when failing to allocate workqueues, we fail to unregister the
           netdev before calling free_netdev()
       (b) even if we do try to unregister the netdev, we're still holding the
           rtnl lock, so the device never properly unregistered; we'll be at
           state NETREG_UNREGISTERING, and then hit free_netdev()'s:
      	BUG_ON(dev->reg_state != NETREG_UNREGISTERED);
       (c) we're allocating some dependent resources (e.g., DFS workqueues)
           after we've registered the interface; this may or may not cause
           problems, but it's good practice to allocate these before registering
       (d) we're not even trying to unwind anything when mwifiex_send_cmd() or
           mwifiex_sta_init_cmd() fail
      
      To fix these issues, let's:
      
       * add a stacked set of error handling labels, to keep error handling
         consistent and properly ordered (resolving (a) and (d))
       * move the workqueue allocations before the registration (to resolve
         (c); also resolves (b) by avoiding error cases where we have to
         unregister)
      
      [Incidentally, it's pretty easy to interrupt the alloc_workqueue() in,
      e.g., the following:
      
        iw phy phy0 interface add mlan0 type station
      
      by sending it SIGTERM.]
      
      This bugfix covers commits like commit 7d652034 ("mwifiex: channel
      switch support for mwifiex"), but parts of this bug exist all the way
      back to the introduction of dynamic interface handling in commit
      93a1df48 ("mwifiex: add cfg80211 handlers add/del_virtual_intf").
      
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarBrian Norris <briannorris@chromium.org>
      Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
      8535107a
    • Brian Norris's avatar
      mwifiex: pcie: de-duplicate buffer allocation code · d41bf5c1
      Brian Norris authored
      This code was duplicated as part of the PCIe FLR code added to this
      driver. Let's de-duplicate it to:
      
       * make things easier to read (mwifiex_pcie_free_buffers() now has a
         corresponding mwifiex_pcie_alloc_buffers())
       * reduce likelihood of bugs
       * make error logging equally verbose
       * save lines of code!
      
      Also drop some of the commentary that isn't really needed.
      Signed-off-by: default avatarBrian Norris <briannorris@chromium.org>
      Signed-off-by: default avatarKalle Valo <kvalo@codeaurora.org>
      d41bf5c1
  2. 18 May, 2017 37 commits