On Thu, 2014-02-20 at 08:18 +0200, Emmanuel Grumbach wrote: > There is a race between the Tx path and the STA wake up > flow. > If a station is asleep, mac80211 buffers frames, but up to > a certain limit. When this limit is reached, mac80211 begins > to drop the oldest buffered frames. This is done in the Tx > path. > When a station wakes up, mac80211 will go over the buffered > frames list and send them. > Since these two flows can run concurrently, we can get to a > situation where the buffered frame list is emptied after the > Tx path checked the length of the list. In that case the Tx > path would get a NULL skb and crash.
I think you should rewrite the commit log - the real fix now isn't really that it can't crash any more, I'd see that as a side effect of the below: > This is only a noisy consequence of another bigger issue: > since ieee80211_tx_h_unicast_ps_buf is not synced against > ieee80211_sta_ps_deliver_wakeup, Tx path could think that > the STA is still sleeping and put the packet on the PS skb > list while in fact the STA is awake already. In this case, > the frame would sit on the PS skb list until the next time > the STA wakes up and then, it'd be sent out of order. > @@ -486,6 +499,7 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data > *tx) > ieee80211_free_txskb(&local->hw, old); > } else > tx->local->total_ps_buffered++; > + spin_unlock(&sta->lock); That needs to be after adding the frame to the queue. johannes -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
