All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: Justin Piszcz <jpiszcz@lucidpixels.com>,
	Ivo van Doorn <IvDoorn@gmail.com>,
	Gertjan van Wingerde <gwingerde@gmail.com>,
	Helmut Schaa <helmut.schaa@googlemail.com>,
	linux-wireless@vger.kernel.org
Subject: [PATCH] rt2x00: rt2800usb: fix races in tx queue
Date: Thu, 4 Aug 2011 14:46:54 +0200	[thread overview]
Message-ID: <20110804124653.GB5739@redhat.com> (raw)

We can perform parallel putting new entries in a queue
(rt2x00queue_write_tx_frame()) and removing entries after finishing
transmitting (rt2800usb_work_txdone()). There are cases when txdone
may process an entry that was not fully send and nullify entry->skb.
What in a result crash the kernel in rt2800usb_write_tx_desc() or
rt2800usb_get_txwi() or other place where entry->skb is used.

To fix stop processing entries in txdone, which flags do not indicate
transition was finished.

Also change flags as soon as we get confirmation transmision is done.

Reported-and-tested-by: Justin Piszcz <jpiszcz@lucidpixels.com>
Cc: stable@kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/rt2x00/rt2800usb.c |   36 ++++++++++++++++++++++--------
 drivers/net/wireless/rt2x00/rt2x00usb.c |    6 ++--
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 5075593..edd0518 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -500,14 +500,14 @@ static bool rt2800usb_txdone_entry_check(struct queue_entry *entry, u32 reg)
 	return true;
 }
 
-static void rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
+static int rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
 {
 	struct data_queue *queue;
 	struct queue_entry *entry;
 	u32 reg;
 	u8 qid;
 
-	while (kfifo_get(&rt2x00dev->txstatus_fifo, &reg)) {
+	while (kfifo_peek(&rt2x00dev->txstatus_fifo, &reg)) {
 
 		/* TX_STA_FIFO_PID_QUEUE is a 2-bit field, thus
 		 * qid is guaranteed to be one of the TX QIDs
@@ -517,25 +517,39 @@ static void rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
 		if (unlikely(!queue)) {
 			WARNING(rt2x00dev, "Got TX status for an unavailable "
 					   "queue %u, dropping\n", qid);
-			continue;
+			goto next_reg;
 		}
 
 		/*
 		 * Inside each queue, we process each entry in a chronological
 		 * order. We first check that the queue is not empty.
 		 */
-		entry = NULL;
-		while (!rt2x00queue_empty(queue)) {
+		while (1) {
+			entry = NULL;
+
+			if (rt2x00queue_empty(queue))
+				break;
+
 			entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
+
+			if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
+			    !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags)) {
+				WARNING(rt2x00dev, "Data pending for entry %u"
+					"in queue %u\n", entry->entry_idx, qid);
+				return 1;
+			}
+
 			if (rt2800usb_txdone_entry_check(entry, reg))
 				break;
 		}
 
-		if (!entry || rt2x00queue_empty(queue))
-			break;
-
-		rt2800_txdone_entry(entry, reg);
+		if (entry)
+			rt2800_txdone_entry(entry, reg);
+next_reg:
+		kfifo_skip(&rt2x00dev->txstatus_fifo);
 	}
+
+	return 0;
 }
 
 static void rt2800usb_work_txdone(struct work_struct *work)
@@ -545,7 +559,8 @@ static void rt2800usb_work_txdone(struct work_struct *work)
 	struct data_queue *queue;
 	struct queue_entry *entry;
 
-	rt2800usb_txdone(rt2x00dev);
+	if (rt2800usb_txdone(rt2x00dev))
+		goto out;
 
 	/*
 	 * Process any trailing TX status reports for IO failures,
@@ -569,6 +584,7 @@ static void rt2800usb_work_txdone(struct work_struct *work)
 		}
 	}
 
+out:
 	/*
 	 * The hw may delay sending the packet after DMA complete
 	 * if the medium is busy, thus the TX_STA_FIFO entry is
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index b6b4542..46c6d36 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -265,14 +265,14 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
 	if (!test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
 		return;
 
-	if (rt2x00dev->ops->lib->tx_dma_done)
-		rt2x00dev->ops->lib->tx_dma_done(entry);
-
 	/*
 	 * Report the frame as DMA done
 	 */
 	rt2x00lib_dmadone(entry);
 
+	if (rt2x00dev->ops->lib->tx_dma_done)
+		rt2x00dev->ops->lib->tx_dma_done(entry);
+
 	/*
 	 * Check if the frame was correctly uploaded
 	 */
-- 
1.7.1


             reply	other threads:[~2011-08-04 12:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-04 12:46 Stanislaw Gruszka [this message]
2011-08-06 11:06 ` [PATCH] rt2x00: rt2800usb: fix races in tx queue Ivo Van Doorn
2011-08-08  9:29   ` Stanislaw Gruszka
2011-08-08  9:35     ` [PATCH v2] " Stanislaw Gruszka
2011-08-08 20:55       ` Gertjan van Wingerde
2011-08-09  9:50         ` Stanislaw Gruszka
2011-08-09 11:26           ` Stanislaw Gruszka
2011-08-09 15:45             ` Stanislaw Gruszka
2011-08-10 10:39               ` Stanislaw Gruszka
2011-08-10 13:28                 ` Stanislaw Gruszka
2011-08-08  9:43     ` [PATCH] " Ivo Van Doorn
2011-08-08 14:28       ` Stanislaw Gruszka
2011-08-08 20:45       ` Gertjan van Wingerde
2011-08-09 10:01         ` Stanislaw Gruszka

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=20110804124653.GB5739@redhat.com \
    --to=sgruszka@redhat.com \
    --cc=IvDoorn@gmail.com \
    --cc=gwingerde@gmail.com \
    --cc=helmut.schaa@googlemail.com \
    --cc=jpiszcz@lucidpixels.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.