wil6210: fix race conditions between TX send and completion
authorMaya Erez <qca_merez@qca.qualcomm.com>
Mon, 16 May 2016 19:23:30 +0000 (22:23 +0300)
committerKalle Valo <kvalo@qca.qualcomm.com>
Sat, 28 May 2016 08:19:09 +0000 (11:19 +0300)
There are 2 possible race conditions, both are solved by addition of
memory barrier:
1. wil_tx_complete reads the swhead to determine if the vring is
empty. In case the swhead was updated before the descriptor update
was performed in __wil_tx_vring/__wil_tx_vring_tso, the completion
loop will not end and as the DU bit may still be set from a previous
run, this skb can be handled as completed before it was sent, which
will lead to double free of the same SKB.
2. __wil_tx_vring/__wil_tx_vring_tso calculate the number of available
descriptors according to the swtail. In case the swtail is updated
before memset of ctx to zero is completed, we can handle this
descriptor while later on ctx is zeroed.

Signed-off-by: Maya Erez <qca_merez@qca.qualcomm.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
drivers/net/wireless/ath/wil6210/txrx.c

index a4e43796addbd8282584cf2e5fbc324a68aa5bac..fa6ea24a1309ed5b35e5eaaec0abb06f96b86cff 100644 (file)
@@ -1551,6 +1551,13 @@ static int __wil_tx_vring_tso(struct wil6210_priv *wil, struct vring *vring,
                             vring_index, used, used + descs_used);
        }
 
+       /* Make sure to advance the head only after descriptor update is done.
+        * This will prevent a race condition where the completion thread
+        * will see the DU bit set from previous run and will handle the
+        * skb before it was completed.
+        */
+       wmb();
+
        /* advance swhead */
        wil_vring_advance_head(vring, descs_used);
        wil_dbg_txrx(wil, "TSO: Tx swhead %d -> %d\n", swhead, vring->swhead);
@@ -1691,6 +1698,13 @@ static int __wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
                             vring_index, used, used + nr_frags + 1);
        }
 
+       /* Make sure to advance the head only after descriptor update is done.
+        * This will prevent a race condition where the completion thread
+        * will see the DU bit set from previous run and will handle the
+        * skb before it was completed.
+        */
+       wmb();
+
        /* advance swhead */
        wil_vring_advance_head(vring, nr_frags + 1);
        wil_dbg_txrx(wil, "Tx[%2d] swhead %d -> %d\n", vring_index, swhead,
@@ -1914,6 +1928,12 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
                                wil_consume_skb(skb, d->dma.error == 0);
                        }
                        memset(ctx, 0, sizeof(*ctx));
+                       /* Make sure the ctx is zeroed before updating the tail
+                        * to prevent a case where wil_tx_vring will see
+                        * this descriptor as used and handle it before ctx zero
+                        * is completed.
+                        */
+                       wmb();
                        /* There is no need to touch HW descriptor:
                         * - ststus bit TX_DMA_STATUS_DU is set by design,
                         *   so hardware will not try to process this desc.,