1. 19 5月, 2017 12 次提交
    • B
      mwifiex: pcie: stop setting/clearing 'surprise_removed' · 68efd038
      Brian Norris 提交于
      These are already handled by mwifiex_shutdown_sw() and
      mwifiex_reinit_sw(). Ideally, we'll kill the flag entirely eventually,
      as I suspect it breeds race conditions.
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      68efd038
    • B
      mwifiex: pcie: remove useless pdev check · fa4651e1
      Brian Norris 提交于
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      fa4651e1
    • B
      mwifiex: 11h: drop unnecessary check for '!priv' · 7ade530e
      Brian Norris 提交于
      These pointers are retrieved via container_of(). There's no way they are
      NULL.
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      7ade530e
    • B
      mwifiex: remove redundant 'adapter' check in mwifiex_adapter_cleanup · 71708627
      Brian Norris 提交于
      We're using 'adapter' right before calling this. Stop being
      unnecessarily paranoid.
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      71708627
    • B
      mwifiex: remove useless 'mwifiex_lock' · bc69ca39
      Brian Norris 提交于
      If mwifiex_shutdown_drv() is racing with another mwifiex_shutdown_drv(),
      we *really* have problems. Kill the lock.
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      bc69ca39
    • B
      mwifiex: don't leak stashed beacon buffer on reset · 6eb2d002
      Brian Norris 提交于
      When removing or resetting an mwifiex device, we don't remember to free
      the saved beacon buffer. Use the (somewhat misleadingly-named)
      mwifiex_free_priv() helper to handle this.
      
      Noticed by kmemleak during tests:
      
      echo 1 > /sys/bus/pci/devices/.../reset
      
      unreferenced object 0xffffffc09d034a00 (size 256):
      ...
        backtrace:
          [<ffffffc0003cdce4>] create_object+0x228/0x3c4
          [<ffffffc000c0b9d8>] kmemleak_alloc+0x54/0x88
          [<ffffffc0003c0848>] __kmalloc+0x1cc/0x2dc
          [<ffffffbffc1500c4>] mwifiex_save_curr_bcn+0x80/0x308 [mwifiex]
          [<ffffffbffc1516b8>] mwifiex_ret_802_11_associate+0x4ec/0x5fc [mwifiex]
          [<ffffffbffc15da90>] mwifiex_process_sta_cmdresp+0xaf8/0x1fa4 [mwifiex]
          [<ffffffbffc1411e0>] mwifiex_process_cmdresp+0x40c/0x510 [mwifiex]
          [<ffffffbffc13b8f4>] mwifiex_main_process+0x4a4/0xb00 [mwifiex]
          [<ffffffbffc13bf84>] mwifiex_main_work_queue+0x34/0x40 [mwifiex]
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      6eb2d002
    • B
      mwifiex: don't drop lock between list-retrieval / list-deletion · 0f13acf0
      Brian Norris 提交于
      mwifiex_exec_next_cmd() seems to have a classic TOCTOU race, where we
      drop the list lock in between retrieving the next command and deleting
      it from the list. This potentially leaves room for someone else to also
      retrieve / steal this node from the list (e.g.,
      mwifiex_cancel_all_pending_cmd()).
      
      Let's keep holding the lock while we do our 'ps_state' sanity checks.
      There should be no harm in continuing to hold this lock for a bit more.
      
      Noticed only by code inspection.
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      0f13acf0
    • D
      mwifiex: Add locking to mwifiex_11n_delba · 09bdb650
      Douglas Anderson 提交于
      The mwifiex_11n_delba() function walked the rx_reorder_tbl_ptr without
      holding the lock, which was an obvious violation.
      
      Grab the lock.
      
      NOTE: we hold the lock while calling mwifiex_send_delba().  There's also
      several callers in 11n_rxreorder.c that hold the lock and the comments
      in the struct sound just like very other list/lock pair -- as if the
      lock should definitely be help for all operations like this.
      Signed-off-by: NDouglas Anderson <dianders@chromium.org>
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      09bdb650
    • D
      mwifiex: Don't release cmd_pending_q_lock while iterating · 90ad0be8
      Douglas Anderson 提交于
      Just like in the previous patch ("mwifiex: Don't release
      tx_ba_stream_tbl_lock while iterating"), in
      mwifiex_cancel_all_pending_cmd() we were itearting over a list protected
      by a spinlock.  Again, it is not safe to release the spinlock while
      iterating.  Don't do it.
      
      Luckily in this case there should be no need to release the spinlock.
      This is evidenced by:
      
      1. The only function called while the spinlock was released was
         mwifiex_recycle_cmd_node()
      2. Aside from atomic functions (which are safe to call), the only
         function called by mwifiex_recycle_cmd_node() was
         mwifiex_insert_cmd_to_free_q().
      3. It can be seen in mwifiex_cancel_pending_scan_cmd() that it's OK to
         call mwifiex_insert_cmd_to_free_q() while holding a different
         spinlock (scan_pending_q_lock), so in general holding a spinlock
         should be OK.
      4. It doesn't appear that mwifiex_insert_cmd_to_free_q() has any
         interaction with the cmd_pending_q_lock
      
      No known bugs are fixed with this change, but as with other similar
      changes this could fix random list corruption.
      Signed-off-by: NDouglas Anderson <dianders@chromium.org>
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      90ad0be8
    • D
      mwifiex: Don't release tx_ba_stream_tbl_lock while iterating · e0b636e5
      Douglas Anderson 提交于
      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: NDouglas Anderson <dianders@chromium.org>
      Signed-off-by: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      e0b636e5
    • B
      mwifiex: fixup error cases in mwifiex_add_virtual_intf() · 8535107a
      Brian Norris 提交于
      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: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      8535107a
    • B
      mwifiex: pcie: de-duplicate buffer allocation code · d41bf5c1
      Brian Norris 提交于
      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: NBrian Norris <briannorris@chromium.org>
      Signed-off-by: NKalle Valo <kvalo@codeaurora.org>
      d41bf5c1
  2. 18 5月, 2017 28 次提交