Re: [01/11] mwifiex: fixup error cases in mwifiex_add_virtual_intf()

[Kalle Valo]

Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
> 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 7d652034d1a0 ("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
> 93a1df48d224 ("mwifiex: add cfg80211 handlers add/del_virtual_intf").
> 
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>

11 patches applied to wireless-drivers-next.git, thanks.

8535107aa4ef mwifiex: fixup error cases in mwifiex_add_virtual_intf()
e0b636e5ee15 mwifiex: Don't release tx_ba_stream_tbl_lock while iterating
90ad0be83676 mwifiex: Don't release cmd_pending_q_lock while iterating
09bdb6500551 mwifiex: Add locking to mwifiex_11n_delba
0f13acf0c612 mwifiex: don't drop lock between list-retrieval / list-deletion
6eb2d002d4ea mwifiex: don't leak stashed beacon buffer on reset
bc69ca391eff mwifiex: remove useless 'mwifiex_lock'
7170862738dc mwifiex: remove redundant 'adapter' check in mwifiex_adapter_cleanup
7ade530e7384 mwifiex: 11h: drop unnecessary check for '!priv'
fa4651e12ae8 mwifiex: pcie: remove useless pdev check
68efd0386988 mwifiex: pcie: stop setting/clearing 'surprise_removed'

-- 
https://patchwork.kernel.org/patch/9724599/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches