On Mon, 2020-04-06 at 17:51 +0530, Sumit Garg wrote: > A race condition leading to a kernel crash is observed during invocation > of ieee80211_register_hw() on a dragonboard410c device having wcn36xx > driver built as a loadable module along with a wifi manager in user-space > waiting for a wifi device (wlanX) to be active. > > Sequence diagram for a particular kernel crash scenario: > > user-space ieee80211_register_hw() RX IRQ > +++++++++++++++++++++++++++++++++++++++++++++ > | | | > |<---wlan0---wiphy_register() | > |----start wlan0---->| | > | |<---IRQ---(RX packet) > | Kernel crash | > | due to unallocated | > | workqueue. | > | | | > | alloc_ordered_workqueue() | > | | | > | Misc wiphy init. | > | | | > | ieee80211_if_add() | > | | | > > As evident from above sequence diagram, this race condition isn't specific > to a particular wifi driver but rather the initialization sequence in > ieee80211_register_hw() needs to be fixed. Indeed, oops. > So re-order the initialization > sequence and the updated sequence diagram would look like: > > user-space ieee80211_register_hw() RX IRQ > +++++++++++++++++++++++++++++++++++++++++++++ > | | | > | alloc_ordered_workqueue() | > | | | > | Misc wiphy init. | > | | | > |<---wlan0---wiphy_register() | > |----start wlan0---->| | > | |<---IRQ---(RX packet) > | | | > | ieee80211_if_add() | > | | | Makes sense. > @@ -1254,6 +1250,14 @@ int ieee80211_register_hw(struct ieee80211_hw *hw) > local->sband_allocated |= BIT(band); > } > > + rtnl_unlock(); > + > + result = wiphy_register(local->hw.wiphy); > + if (result < 0) > + goto fail_wiphy_register; > + > + rtnl_lock(); I'm a bit worried about this unlock/relock here though. I think we only need the RTNL for the call to ieee80211_init_rate_ctrl_alg() and then later ieee80211_if_add(), so perhaps we can move that a little closer? All the stuff between is really just setting up local stuff, so doesn't really need to worry? johannes
