Commit 0aae511c authored by Reinette Chatre's avatar Reinette Chatre Committed by John W. Linville

iwlwifi: fix potential rx buffer loss

RX handling maintains a few lists that keep track of the RX buffers.
Buffers move from one list to the other as they are used, replenished, and
again made available for usage. In one such instance, when a buffer is used
it enters the "rx_used" list. When buffers are replenished an skb is
attached to the buffer and it is moved to the "rx_free" list. The problem
here is that the buffer is first removed from the "rx_used" list _before_ the
skb is allocated. Thus, if the skb allocation fails this buffer remains
removed from the "rx_used" list and is thus lost for future usage.

Fix this by first allocating the skb before trying to attach it to a list.
We add an additional check to not do this unnecessarily.
Reported-by: default avatarRick Farrington <rickdic@hotmail.com>
Signed-off-by: default avatarReinette Chatre <reinette.chatre@intel.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 392a0baf
...@@ -239,26 +239,22 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority) ...@@ -239,26 +239,22 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority)
struct iwl_rx_queue *rxq = &priv->rxq; struct iwl_rx_queue *rxq = &priv->rxq;
struct list_head *element; struct list_head *element;
struct iwl_rx_mem_buffer *rxb; struct iwl_rx_mem_buffer *rxb;
struct sk_buff *skb;
unsigned long flags; unsigned long flags;
while (1) { while (1) {
spin_lock_irqsave(&rxq->lock, flags); spin_lock_irqsave(&rxq->lock, flags);
if (list_empty(&rxq->rx_used)) { if (list_empty(&rxq->rx_used)) {
spin_unlock_irqrestore(&rxq->lock, flags); spin_unlock_irqrestore(&rxq->lock, flags);
return; return;
} }
element = rxq->rx_used.next;
rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
list_del(element);
spin_unlock_irqrestore(&rxq->lock, flags); spin_unlock_irqrestore(&rxq->lock, flags);
/* Alloc a new receive buffer */ /* Alloc a new receive buffer */
rxb->skb = alloc_skb(priv->hw_params.rx_buf_size + 256, skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
priority); priority);
if (!rxb->skb) { if (!skb) {
IWL_CRIT(priv, "Can not allocate SKB buffers\n"); IWL_CRIT(priv, "Can not allocate SKB buffers\n");
/* We don't reschedule replenish work here -- we will /* We don't reschedule replenish work here -- we will
* call the restock method and if it still needs * call the restock method and if it still needs
...@@ -266,6 +262,20 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority) ...@@ -266,6 +262,20 @@ void iwl_rx_allocate(struct iwl_priv *priv, gfp_t priority)
break; break;
} }
spin_lock_irqsave(&rxq->lock, flags);
if (list_empty(&rxq->rx_used)) {
spin_unlock_irqrestore(&rxq->lock, flags);
dev_kfree_skb_any(skb);
return;
}
element = rxq->rx_used.next;
rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
list_del(element);
spin_unlock_irqrestore(&rxq->lock, flags);
rxb->skb = skb;
/* Get physical address of RB/SKB */ /* Get physical address of RB/SKB */
rxb->real_dma_addr = pci_map_single( rxb->real_dma_addr = pci_map_single(
priv->pci_dev, priv->pci_dev,
......
...@@ -1134,6 +1134,7 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority) ...@@ -1134,6 +1134,7 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority)
struct iwl_rx_queue *rxq = &priv->rxq; struct iwl_rx_queue *rxq = &priv->rxq;
struct list_head *element; struct list_head *element;
struct iwl_rx_mem_buffer *rxb; struct iwl_rx_mem_buffer *rxb;
struct sk_buff *skb;
unsigned long flags; unsigned long flags;
while (1) { while (1) {
...@@ -1143,17 +1144,11 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority) ...@@ -1143,17 +1144,11 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority)
spin_unlock_irqrestore(&rxq->lock, flags); spin_unlock_irqrestore(&rxq->lock, flags);
return; return;
} }
element = rxq->rx_used.next;
rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
list_del(element);
spin_unlock_irqrestore(&rxq->lock, flags); spin_unlock_irqrestore(&rxq->lock, flags);
/* Alloc a new receive buffer */ /* Alloc a new receive buffer */
rxb->skb = skb = alloc_skb(priv->hw_params.rx_buf_size, priority);
alloc_skb(priv->hw_params.rx_buf_size, if (!skb) {
priority);
if (!rxb->skb) {
if (net_ratelimit()) if (net_ratelimit())
IWL_CRIT(priv, ": Can not allocate SKB buffers\n"); IWL_CRIT(priv, ": Can not allocate SKB buffers\n");
/* We don't reschedule replenish work here -- we will /* We don't reschedule replenish work here -- we will
...@@ -1162,6 +1157,19 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority) ...@@ -1162,6 +1157,19 @@ static void iwl3945_rx_allocate(struct iwl_priv *priv, gfp_t priority)
break; break;
} }
spin_lock_irqsave(&rxq->lock, flags);
if (list_empty(&rxq->rx_used)) {
spin_unlock_irqrestore(&rxq->lock, flags);
dev_kfree_skb_any(skb);
return;
}
element = rxq->rx_used.next;
rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
list_del(element);
spin_unlock_irqrestore(&rxq->lock, flags);
rxb->skb = skb;
/* If radiotap head is required, reserve some headroom here. /* If radiotap head is required, reserve some headroom here.
* The physical head count is a variable rx_stats->phy_count. * The physical head count is a variable rx_stats->phy_count.
* We reserve 4 bytes here. Plus these extra bytes, the * We reserve 4 bytes here. Plus these extra bytes, the
......
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