b43: Rewrite DMA Tx status handling sanity checks

This rewrites the error handling policies in the TX status handler.
It tries to be error-tolerant as in "try hard to not crash the machine".
It won't recover from errors (that are bugs in the firmware or driver),
because that's impossible. However, it will return a more or less useful
error message and bail out. It also tries hard to use rate-limited messages
to not flood the syslog in case of a failure.

Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

1 files changed, 54 insertions, 13 deletions

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 18b97c02b8a..027be275e03 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -874,7 +874,7 @@ static void free_all_descbuffers(struct b43_dmaring *ring)
         for (i = 0; i < ring->nr_slots; i++) {
                 desc = ring->ops->idx2desc(ring, i, &meta);
 
-                if (!meta->skb) {
+                if (!meta->skb || b43_dma_ptr_is_poisoned(meta->skb)) {
                         B43_WARN_ON(!ring->tx);
                         continue;
                 }
@@ -926,7 +926,7 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
                                       enum b43_dmatype type)
 {
         struct b43_dmaring *ring;
-        int err;
+        int i, err;
         dma_addr_t dma_test;
 
         ring = kzalloc(sizeof(*ring), GFP_KERNEL);
@@ -941,6 +941,8 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
                              GFP_KERNEL);
         if (!ring->meta)
                 goto err_kfree_ring;
+        for (i = 0; i < ring->nr_slots; i++)
+                ring->meta->skb = B43_DMA_PTR_POISON;
 
         ring->type = type;
         ring->dev = dev;
@@ -1251,11 +1253,13 @@ struct b43_dmaring *parse_cookie(struct b43_wldev *dev, u16 cookie, int *slot)
         case 0x5000:
                 ring = dma->tx_ring_mcast;
                 break;
-        default:
-                B43_WARN_ON(1);
         }
         *slot = (cookie & 0x0FFF);
-        B43_WARN_ON(!(ring && *slot >= 0 && *slot < ring->nr_slots));
+        if (unlikely(!ring || *slot < 0 || *slot >= ring->nr_slots)) {
+                b43dbg(dev->wl, "TX-status contains "
+                       "invalid cookie: 0x%04X\n", cookie);
+                return NULL;
+        }
 
         return ring;
 }
@@ -1494,19 +1498,40 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
         struct b43_dmaring *ring;
         struct b43_dmadesc_generic *desc;
         struct b43_dmadesc_meta *meta;
-        int slot;
+        int slot, firstused;
         bool frame_succeed;
 
         ring = parse_cookie(dev, status->cookie, &slot);
         if (unlikely(!ring))
                 return;
-
         B43_WARN_ON(!ring->tx);
+
+        /* Sanity check: TX packets are processed in-order on one ring.
+         * Check if the slot deduced from the cookie really is the first
+         * used slot. */
+        firstused = ring->current_slot - ring->used_slots + 1;
+        if (firstused < 0)
+                firstused = ring->nr_slots + firstused;
+        if (unlikely(slot != firstused)) {
+                /* This possibly is a firmware bug and will result in
+                 * malfunction, memory leaks and/or stall of DMA functionality. */
+                b43dbg(dev->wl, "Out of order TX status report on DMA ring %d. "
+                       "Expected %d, but got %d\n",
+                       ring->index, firstused, slot);
+                return;
+        }
+
         ops = ring->ops;
         while (1) {
-                B43_WARN_ON(!(slot >= 0 && slot < ring->nr_slots));
+                B43_WARN_ON(slot < 0 || slot >= ring->nr_slots);
                 desc = ops->idx2desc(ring, slot, &meta);
 
+                if (b43_dma_ptr_is_poisoned(meta->skb)) {
+                        b43dbg(dev->wl, "Poisoned TX slot %d (first=%d) "
+                               "on ring %d\n",
+                               slot, firstused, ring->index);
+                        break;
+                }
                 if (meta->skb) {
                         struct b43_private_tx_info *priv_info =
                                 b43_get_priv_tx_info(IEEE80211_SKB_CB(meta->skb));
@@ -1522,7 +1547,14 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
                 if (meta->is_last_fragment) {
                         struct ieee80211_tx_info *info;
 
-                        BUG_ON(!meta->skb);
+                        if (unlikely(!meta->skb)) {
+                                /* This is a scatter-gather fragment of a frame, so
+                                 * the skb pointer must not be NULL. */
+                                b43dbg(dev->wl, "TX status unexpected NULL skb "
+                                       "at slot %d (first=%d) on ring %d\n",
+                                       slot, firstused, ring->index);
+                                break;
+                        }
 
                         info = IEEE80211_SKB_CB(meta->skb);
 
@@ -1540,20 +1572,29 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
 #endif /* DEBUG */
                         ieee80211_tx_status(dev->wl->hw, meta->skb);
 
-                        /* skb is freed by ieee80211_tx_status() */
-                        meta->skb = NULL;
+                        /* skb will be freed by ieee80211_tx_status().
+                         * Poison our pointer. */
+                        meta->skb = B43_DMA_PTR_POISON;
                 } else {
                         /* No need to call free_descriptor_buffer here, as
                          * this is only the txhdr, which is not allocated.
                          */
-                        B43_WARN_ON(meta->skb);
+                        if (unlikely(meta->skb)) {
+                                b43dbg(dev->wl, "TX status unexpected non-NULL skb "
+                                       "at slot %d (first=%d) on ring %d\n",
+                                       slot, firstused, ring->index);
+                                break;
+                        }
                 }
 
                 /* Everything unmapped and free'd. So it's not used anymore. */
                 ring->used_slots--;
 
-                if (meta->is_last_fragment)
+                if (meta->is_last_fragment) {
+                        /* This is the last scatter-gather
+                         * fragment of the frame. We are done. */
                         break;
+                }
                 slot = next_slot(ring, slot);
         }
         if (ring->stopped) {