Commit 2716fd7d authored by Andreas Fenkart's avatar Andreas Fenkart Committed by John W. Linville

mwifiex: hold proper locks when accessing ra_list / bss_prio lists

Not locking ra_list when dequeuing packets creates race conditions.
When adding a packet 'tx_pkts_queued' is modified before setting
highest_priority_queue. If in-between the main loop starts, it will
see a packet queued (tx_pkts_queued > 0) but will not find it, since
max prio is not set yet. Depending on the scheduling, the thread
trying to add the packet could complete and restore the situation.
But this is not something to rely on.

Another race condition exists, if a new packet, exceeding current
max prio is added. If concurrently a packet is dequeued, the newly
set max prio will be overwritten with the value of the dequeued
packet. This can occur, because selecting a packet and modifying
the max prio is not atomic. The result in an infinite loop unless,
a new packet is added that has at least the priority of the hidden
packet.

Same applies to bss_prio_tbl. Forward iteration is no proper
lock-free technique and provides no protection from calls to
list_del. Although BSS are currently not added/removed dynamically,
this must not be the case in the future. Hence always hold proper
locks when accessing those lists.
Signed-off-by: default avatarAndreas Fenkart <andreas.fenkart@streamunlimited.com>
Signed-off-by: default avatarBing Zhao <bzhao@marvell.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 333f6b22
...@@ -685,13 +685,13 @@ mwifiex_wmm_add_buf_txqueue(struct mwifiex_private *priv, ...@@ -685,13 +685,13 @@ mwifiex_wmm_add_buf_txqueue(struct mwifiex_private *priv,
ra_list->total_pkts_size += skb->len; ra_list->total_pkts_size += skb->len;
ra_list->pkt_count++; ra_list->pkt_count++;
atomic_inc(&priv->wmm.tx_pkts_queued);
if (atomic_read(&priv->wmm.highest_queued_prio) < if (atomic_read(&priv->wmm.highest_queued_prio) <
tos_to_tid_inv[tid_down]) tos_to_tid_inv[tid_down])
atomic_set(&priv->wmm.highest_queued_prio, atomic_set(&priv->wmm.highest_queued_prio,
tos_to_tid_inv[tid_down]); tos_to_tid_inv[tid_down]);
atomic_inc(&priv->wmm.tx_pkts_queued);
spin_unlock_irqrestore(&priv->wmm.ra_list_spinlock, flags); spin_unlock_irqrestore(&priv->wmm.ra_list_spinlock, flags);
} }
...@@ -887,19 +887,15 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter, ...@@ -887,19 +887,15 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
struct mwifiex_bss_prio_node *bssprio_node, *bssprio_head; struct mwifiex_bss_prio_node *bssprio_node, *bssprio_head;
struct mwifiex_tid_tbl *tid_ptr; struct mwifiex_tid_tbl *tid_ptr;
atomic_t *hqp; atomic_t *hqp;
int is_list_empty; unsigned long flags_bss, flags_ra;
unsigned long flags;
int i, j; int i, j;
for (j = adapter->priv_num - 1; j >= 0; --j) { for (j = adapter->priv_num - 1; j >= 0; --j) {
spin_lock_irqsave(&adapter->bss_prio_tbl[j].bss_prio_lock, spin_lock_irqsave(&adapter->bss_prio_tbl[j].bss_prio_lock,
flags); flags_bss);
is_list_empty = list_empty(&adapter->bss_prio_tbl[j]
.bss_prio_head); if (list_empty(&adapter->bss_prio_tbl[j].bss_prio_head))
spin_unlock_irqrestore(&adapter->bss_prio_tbl[j].bss_prio_lock, goto skip_prio_tbl;
flags);
if (is_list_empty)
continue;
if (adapter->bss_prio_tbl[j].bss_prio_cur == if (adapter->bss_prio_tbl[j].bss_prio_cur ==
(struct mwifiex_bss_prio_node *) (struct mwifiex_bss_prio_node *)
...@@ -924,21 +920,18 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter, ...@@ -924,21 +920,18 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
hqp = &priv_tmp->wmm.highest_queued_prio; hqp = &priv_tmp->wmm.highest_queued_prio;
for (i = atomic_read(hqp); i >= LOW_PRIO_TID; --i) { for (i = atomic_read(hqp); i >= LOW_PRIO_TID; --i) {
spin_lock_irqsave(&priv_tmp->wmm.
ra_list_spinlock, flags_ra);
tid_ptr = &(priv_tmp)->wmm. tid_ptr = &(priv_tmp)->wmm.
tid_tbl_ptr[tos_to_tid[i]]; tid_tbl_ptr[tos_to_tid[i]];
/* For non-STA ra_list_curr may be NULL */ /* For non-STA ra_list_curr may be NULL */
if (!tid_ptr->ra_list_curr) if (!tid_ptr->ra_list_curr)
continue; goto skip_wmm_queue;
spin_lock_irqsave(&priv_tmp->wmm. if (list_empty(&tid_ptr->ra_list))
ra_list_spinlock, flags); goto skip_wmm_queue;
is_list_empty =
list_empty(&tid_ptr->ra_list);
spin_unlock_irqrestore(&priv_tmp->wmm.
ra_list_spinlock, flags);
if (is_list_empty)
continue;
/* /*
* Always choose the next ra we transmitted * Always choose the next ra we transmitted
...@@ -960,10 +953,8 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter, ...@@ -960,10 +953,8 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
} }
do { do {
is_list_empty = if (!skb_queue_empty(&ptr->skb_head))
skb_queue_empty(&ptr->skb_head); /* holds both locks */
if (!is_list_empty)
goto found; goto found;
/* Get next ra */ /* Get next ra */
...@@ -978,6 +969,11 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter, ...@@ -978,6 +969,11 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
struct mwifiex_ra_list_tbl, struct mwifiex_ra_list_tbl,
list); list);
} while (ptr != head); } while (ptr != head);
skip_wmm_queue:
spin_unlock_irqrestore(&priv_tmp->wmm.
ra_list_spinlock,
flags_ra);
} }
skip_bss: skip_bss:
...@@ -995,14 +991,21 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter, ...@@ -995,14 +991,21 @@ mwifiex_wmm_get_highest_priolist_ptr(struct mwifiex_adapter *adapter,
struct mwifiex_bss_prio_node, struct mwifiex_bss_prio_node,
list); list);
} while (bssprio_node != bssprio_head); } while (bssprio_node != bssprio_head);
skip_prio_tbl:
spin_unlock_irqrestore(&adapter->bss_prio_tbl[j].bss_prio_lock,
flags_bss);
} }
return NULL; return NULL;
found: found:
spin_lock_irqsave(&priv_tmp->wmm.ra_list_spinlock, flags); /* holds bss_prio_lock / ra_list_spinlock */
if (atomic_read(hqp) > i) if (atomic_read(hqp) > i)
atomic_set(hqp, i); atomic_set(hqp, i);
spin_unlock_irqrestore(&priv_tmp->wmm.ra_list_spinlock, flags); spin_unlock_irqrestore(&priv_tmp->wmm.ra_list_spinlock, flags_ra);
spin_unlock_irqrestore(&adapter->bss_prio_tbl[j].bss_prio_lock,
flags_bss);
*priv = priv_tmp; *priv = priv_tmp;
*tid = tos_to_tid[i]; *tid = tos_to_tid[i];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment