[PATCH] b43: Fix DMA TX bounce buffer copying

[PATCH] b43: Fix DMA TX bounce buffer copying

[Michael Buesch]

b43 allocates a bouncebuffer, if the supplied TX skb is in an invalid
memory range for DMA.
However, this is broken in that it fails to copy over some metadata to the
new skb.

This patch fixes three problems:
* Failure to adjust the ieee80211_tx_info pointer to the new buffer.
  This results in a kmemcheck warning.
* Failure to copy the skb cb, which contains ieee80211_tx_info, to the new skb.
  This results in breakage of various TX-status postprocessing (Rate control).
* Failure to transfer the queue mapping.
  This results in the wrong queue being stopped on saturation and can result in queue overflow.

Signed-off-by: Michael Buesch <mb@bu3sch.de>
Tested-by: Christian Casteyde <casteyde.christian@free.fr>

---

Thanks to Johannes for tracking down this hard to find bug and
thanks to Christian for testing.

Larry, I think I remember you reported a strange rate control failure on
one of your cards. Does this patch fix it?

b43legacy:
Note that b43legacy has the same bug and also needs fixing.
Is there a maintainer for b43legacy?

stable:
I'm not sure if it's necessary to send this patch to stable.
The problem is there since the very beginning and it doesn't affect many people.
I'll leave this decision to somebody else, if somebody cares.




---
 drivers/net/wireless/b43/dma.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/b43/dma.c
+++ wireless-testing/drivers/net/wireless/b43/dma.c
@@ -1157,8 +1157,9 @@ struct b43_dmaring *parse_cookie(struct 
 }
 
 static int dma_tx_fragment(struct b43_dmaring *ring,
-			   struct sk_buff *skb)
+			   struct sk_buff **in_skb)
 {
+	struct sk_buff *skb = *in_skb;
 	const struct b43_dma_ops *ops = ring->ops;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	u8 *header;
@@ -1224,8 +1225,14 @@ static int dma_tx_fragment(struct b43_dm
 		}
 
 		memcpy(skb_put(bounce_skb, skb->len), skb->data, skb->len);
+		memcpy(bounce_skb->cb, skb->cb, sizeof(skb->cb));
+		bounce_skb->dev = skb->dev;
+		skb_set_queue_mapping(bounce_skb, skb_get_queue_mapping(skb));
+		info = IEEE80211_SKB_CB(bounce_skb);
+
 		dev_kfree_skb_any(skb);
 		skb = bounce_skb;
+		*in_skb = bounce_skb;
 		meta->skb = skb;
 		meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1);
 		if (b43_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) {
@@ -1355,7 +1362,11 @@ int b43_dma_tx(struct b43_wldev *dev, st
 	 * static, so we don't need to store it per frame. */
 	ring->queue_prio = skb_get_queue_mapping(skb);
 
-	err = dma_tx_fragment(ring, skb);
+	/* dma_tx_fragment might reallocate the skb, so invalidate pointers pointing
+	 * into the skb data or cb now. */
+	hdr = NULL;
+	info = NULL;
+	err = dma_tx_fragment(ring, &skb);
 	if (unlikely(err == -ENOKEY)) {
 		/* Drop this packet, as we don't have the encryption key
 		 * anymore and must not transmit it unencrypted. */

-- 
Greetings, Michael.

Re: [PATCH] b43: Fix DMA TX bounce buffer copying

[Larry Finger]

Michael Buesch wrote:
> b43 allocates a bouncebuffer, if the supplied TX skb is in an invalid
> memory range for DMA.
> However, this is broken in that it fails to copy over some metadata to the
> new skb.
> 
> This patch fixes three problems:
> * Failure to adjust the ieee80211_tx_info pointer to the new buffer.
>   This results in a kmemcheck warning.
> * Failure to copy the skb cb, which contains ieee80211_tx_info, to the new skb.
>   This results in breakage of various TX-status postprocessing (Rate control).
> * Failure to transfer the queue mapping.
>   This results in the wrong queue being stopped on saturation and can result in queue overflow.
> 
> Signed-off-by: Michael Buesch <mb@bu3sch.de>
> Tested-by: Christian Casteyde <casteyde.christian@free.fr>
> 
> ---
> 
> Thanks to Johannes for tracking down this hard to find bug and
> thanks to Christian for testing.
> 
> Larry, I think I remember you reported a strange rate control failure on
> one of your cards. Does this patch fix it?
> 
> b43legacy:
> Note that b43legacy has the same bug and also needs fixing.
> Is there a maintainer for b43legacy?

Michael,

There was not really a rate-control problem - just a difficulty with
the operator. I was making a transmit test, then interrogating the
rate from the same console. By that time, the rate was back to 1 Mb/s
for control messages. When I looped the transmit tests, and looked at
the rate in a separate console, the code was doing mostly what it
should. Mostly because the success rate for the LP PHY does not
satisfy the assumptions that mistrel makes. One gets a higher
throughput with a fixed rate, but that will improve as power control
is implemented.

There is no maintainer for b43legacy. I have ported your patch for b43
to legacy and I will be testing and submitting it.

Larry