+Emmanuel
Interesting. We just ran into the same issue as well.
> + struct sk_buff *old_skb = NULL;
> + unsigned long flags;
>
> ps_dbg(sta->sdata, "STA %pM aid %d: PS buffer for AC %d\n",
> sta->sta.addr, sta->sta.aid, ac);
> if (tx->local->total_ps_buffered >= TOTAL_MAX_TX_BUFFER)
> purge_old_ps_buffers(tx->local);
> if (skb_queue_len(&sta->ps_tx_buf[ac]) >= STA_MAX_TX_BUFFER) {
> - struct sk_buff *old = skb_dequeue(&sta->ps_tx_buf[ac]);
> + spin_lock_irqsave(&sta->ps_tx_buf[ac].lock, flags);
> + /* queue could be modified, recheck length with lock taken */
> + if (skb_queue_len(&sta->ps_tx_buf[ac]) >= STA_MAX_TX_BUFFER)
> + old_skb = __skb_dequeue(&sta->ps_tx_buf[ac]);
> + spin_unlock_irqrestore(&sta->ps_tx_buf[ac].lock, flags);
> + }
> + if (old_skb) {
I think that's pointless, you can just say "if (old)" instead as the
dequeue would return NULL.
In any case, while this solves the crash which is a good thing, it still
leaves the code buggy. This crash seems to occur in the following racy
scenario:
* station is sleeping
* frame TX to station begins
* station wakes up
* frame TX goes into the queue length check, finds long queue
* pending frames are transmitted
* queue is now empty
* old = skb_dequeue() returns NULL
* *kaboom*
The problem is that you're just fixing the "*kaboom*" part, so the code
will continue like this:
* old is NULL
* no kaboom
* new frame is queued on ps_tx_buf queue
* frame never gets transmitted
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
