On Thu, 2017-01-12 at 15:28 +0100, Michal Kazior wrote: > Station structure is considered as not uploaded > (to driver) until drv_sta_state() finishes. This > call is however done after the structure is > attached to mac80211 internal lists and hashes. > This means mac80211 can lookup (and use) station > structure before it is uploaded to a driver. > > If this happens (structure exists, but > sta->uploaded is false) fast_tx path can still be > taken. Deep in the fastpath call the sta->uploaded > is checked against to derive "pubsta" argument for > ieee80211_get_txq(). If sta->uploaded is false > (and sta is actually non-NULL) ieee80211_get_txq() > effectively downgraded to vif->txq. > > At first glance this may look innocent but coerces > mac80211 into a state that is almost guaranteed > (codel may drop offending skb) to crash because a > station-oriented skb gets queued up on > vif-oriented txq. The ieee80211_tx_dequeue() ends > up looking at info->control.flags and tries to use > txq->sta which in the fail case is NULL. > > It's probably pointless to pretend one can > downgrade skb from sta-txq to vif-txq. Ok. I understand things until this point, more or less. What I don't understand - and you haven't really described - is how the changes fix it? Could you resend with a paragraph added that explains that? Also, you're adding a test: > if (sta && !sta->uploaded) but couldn't do move that into the existing "if (sta)" block? Everything before that only ever returns NULL anyway. johannes
