Re: [PATCH] mac80211: prevent skb/txq mismatch

[Johannes Berg <johannes@sipsolutions.net>]

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