linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maya Erez <qca_merez@qca.qualcomm.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: Maya Erez <qca_merez@qca.qualcomm.com>,
	linux-wireless@vger.kernel.org, wil6210@qca.qualcomm.com
Subject: [PATCH 1/6] wil6210: fix race conditions between TX send and completion
Date: Mon, 16 May 2016 22:23:30 +0300	[thread overview]
Message-ID: <1463426615-15523-2-git-send-email-qca_merez@qca.qualcomm.com> (raw)
In-Reply-To: <1463426615-15523-1-git-send-email-qca_merez@qca.qualcomm.com>

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>
---
 drivers/net/wireless/ath/wil6210/txrx.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index a4e4379..fa6ea24 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -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.,
-- 
1.8.5.2


  reply	other threads:[~2016-05-16 19:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-16 19:23 [PATCH 0/6] wil6210 patches Maya Erez
2016-05-16 19:23 ` Maya Erez [this message]
2016-05-28  8:19   ` [1/6] wil6210: fix race conditions between TX send and completion Kalle Valo
2016-05-16 19:23 ` [PATCH 2/6] wil6210: guarantee safe access to rx descriptors shared memory Maya Erez
2016-05-16 19:23 ` [PATCH 3/6] wil6210: protect wil_vring_fini_tx in parallel to tx completions Maya Erez
2016-05-16 19:23 ` [PATCH 4/6] wil6210: fix dma mapping error cleanup in __wil_tx_vring_tso Maya Erez
2016-05-16 19:23 ` [PATCH 5/6] wil6210: add pm_notify handling Maya Erez
2016-05-16 19:23 ` [PATCH 6/6] wil6210: align wil log functions to wil_dbg_ratelimited implementation Maya Erez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1463426615-15523-2-git-send-email-qca_merez@qca.qualcomm.com \
    --to=qca_merez@qca.qualcomm.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=wil6210@qca.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).