Re: [PATCH] rt2x00: rt2800: fix zeroing skb structure

From: Stanislaw Gruszka
Date: Wed Aug 03 2011 - 12:01:01 EST

On Sat, Jul 30, 2011 at 01:07:11PM -0400, Justin Piszcz wrote:
> Here you go, crash 2:
> http://home.comcast.net/~jpiszcz/20110730/2630-rt2800usb-crash2p1.jpg
> http://home.comcast.net/~jpiszcz/20110730/2630-rt2800usb-crash2p2.jpg

Here is next (draw) patch to test:

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 2a6aa85..2d662f3 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -725,14 +725,14 @@ void rt2800_txdone_entry(struct queue_entry *entry, u32 status)
}
EXPORT_SYMBOL_GPL(rt2800_txdone_entry);

-void rt2800_txdone(struct rt2x00_dev *rt2x00dev)
+int rt2800_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
@@ -742,25 +742,38 @@ void rt2800_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)) {
+ ERROR(rt2x00dev, "Data pending\n");
+ return 1;
+ }
+
if (rt2800_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:
+ if (kfifo_get(&rt2x00dev->txstatus_fifo, &reg) != 1)
+ ERROR(rt2x00dev, "BUG on kfifo");
}
+
+ return 0;
}
EXPORT_SYMBOL_GPL(rt2800_txdone);

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.h b/drivers/net/wireless/rt2x00/rt2800lib.h
index f2d1594..54d0d14 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.h
+++ b/drivers/net/wireless/rt2x00/rt2800lib.h
@@ -152,7 +152,7 @@ void rt2800_write_tx_data(struct queue_entry *entry,
struct txentry_desc *txdesc);
void rt2800_process_rxwi(struct queue_entry *entry, struct rxdone_entry_desc *txdesc);

-void rt2800_txdone(struct rt2x00_dev *rt2x00dev);
+int rt2800_txdone(struct rt2x00_dev *rt2x00dev);
void rt2800_txdone_entry(struct queue_entry *entry, u32 status);

void rt2800_write_beacon(struct queue_entry *entry, struct txentry_desc *txdesc);
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index ba82c97..a8a9f79 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -464,7 +464,8 @@ static void rt2800usb_work_txdone(struct work_struct *work)
struct data_queue *queue;
struct queue_entry *entry;

- rt2800_txdone(rt2x00dev);
+ if (rt2800_txdone(rt2x00dev))
+ goto out;

/*
* Process any trailing TX status reports for IO failures,
@@ -488,6 +489,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/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index ab8c16f..7635014 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -784,6 +784,57 @@ bool rt2x00queue_for_each_entry(struct data_queue *queue,
}
EXPORT_SYMBOL_GPL(rt2x00queue_for_each_entry);

+static void rt2x00queue_validate(struct data_queue *queue)
+{
+ int idx0, idx1, idx2;
+ int tmp = 0;
+ int u;
+
+ switch (queue->qid) {
+ case QID_AC_VO:
+ case QID_AC_VI:
+ case QID_AC_BE:
+ case QID_AC_BK:
+ goto do_validate;
+ default:
+ return;
+ }
+
+do_validate:
+
+ idx0 = queue->index[2];
+ idx1 = queue->index[1];
+ idx2 = queue->index[0];
+
+ tmp = idx0 + queue->length;
+ if (tmp >= queue->limit)
+ tmp -= queue->limit;
+
+ if (tmp != idx2) {
+ u = 0;
+ goto print;
+ }
+
+ if (idx2 >= idx0) {
+ u = 1;
+ if (idx1 < idx0 || idx1 > idx2)
+ goto print;
+ } else {
+ bool check = (idx1 >= idx0 && idx1 < queue->limit) ||
+ (idx1 >= 0 && idx1 <= idx2);
+
+ u = 2;
+ if (!check)
+ goto print;
+ }
+
+ return;
+
+print:
+ printk(KERN_CRIT "%s %d idx(%d, %d, %d) tmp %d\n", __func__, u, idx2, idx1, idx0, tmp);
+ BUG_ON(1);
+}
+
struct queue_entry *rt2x00queue_get_entry(struct data_queue *queue,
enum queue_index index)
{
@@ -800,6 +851,7 @@ struct queue_entry *rt2x00queue_get_entry(struct data_queue *queue,

entry = &queue->entries[queue->index[index]];

+ rt2x00queue_validate(queue);
spin_unlock_irqrestore(&queue->index_lock, irqflags);

return entry;
@@ -832,6 +884,7 @@ void rt2x00queue_index_inc(struct queue_entry *entry, enum queue_index index)
queue->count++;
}

+ rt2x00queue_validate(queue);
spin_unlock_irqrestore(&queue->index_lock, irqflags);
}

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 8f90f62..de3720f 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
*/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/