linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
@ 2010-12-02 17:44 Helmut Schaa
  2010-12-02 17:46 ` Johannes Berg
  2010-12-07 19:49 ` John W. Linville
  0 siblings, 2 replies; 6+ messages in thread
From: Helmut Schaa @ 2010-12-02 17:44 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Helmut Schaa, Johannes Berg

mac80211 doesn't handle shared skbs correctly at the moment. As a result
a possible resize can trigger a BUG in pskb_expand_head.

[  676.030000] Kernel bug detected[#1]:
[  676.030000] Cpu 0
[  676.030000] $ 0   : 00000000 00000000 819662ff 00000002
[  676.030000] $ 4   : 81966200 00000020 00000000 00000020
[  676.030000] $ 8   : 819662e0 800043c0 00000002 00020000
[  676.030000] $12   : 3b9aca00 00000000 00000000 00470000
[  676.030000] $16   : 80ea2000 00000000 00000000 00000000
[  676.030000] $20   : 818aa200 80ea2018 80ea2000 00000008
[  676.030000] $24   : 00000002 800ace5c                  
[  676.030000] $28   : 8199a000 8199bd20 81938f88 80f180d4
[  676.030000] Hi    : 0000026e
[  676.030000] Lo    : 0000757e
[  676.030000] epc   : 801245e4 pskb_expand_head+0x44/0x1d8
[  676.030000]     Not tainted
[  676.030000] ra    : 80f180d4 ieee80211_skb_resize+0xb0/0x114 [mac80211]
[  676.030000] Status: 1000a403    KERNEL EXL IE 
[  676.030000] Cause : 10800024
[  676.030000] PrId  : 0001964c (MIPS 24Kc)
[  676.030000] Modules linked in: mac80211_hwsim rt2800lib rt2x00soc rt2x00pci rt2x00lib mac80211 crc_itu_t crc_ccitt cfg80211 compat arc4 aes_generic deflate ecb cbc [last unloaded: rt2800pci]
[  676.030000] Process kpktgend_0 (pid: 97, threadinfo=8199a000, task=81879f48, tls=00000000)
[  676.030000] Stack : ffffffff 00000000 00000000 00000014 00000004 80ea2000 00000000 00000000
[  676.030000]         818aa200 80f180d4 ffffffff 0000000a 81879f78 81879f48 81879f48 00000018
[  676.030000]         81966246 80ea2000 818432e0 80f1a420 80203050 81814d98 00000001 81879f48
[  676.030000]         81879f48 00000018 81966246 818432e0 0000001a 8199bdd4 0000001c 80f1b72c
[  676.030000]         80203020 8001292c 80ef4aa2 7f10b55d 801ab5b8 81879f48 00000188 80005c90
[  676.030000]         ...
[  676.030000] Call Trace:
[  676.030000] [<801245e4>] pskb_expand_head+0x44/0x1d8
[  676.030000] [<80f180d4>] ieee80211_skb_resize+0xb0/0x114 [mac80211]
[  676.030000] [<80f1a420>] ieee80211_xmit+0x150/0x22c [mac80211]
[  676.030000] [<80f1b72c>] ieee80211_subif_start_xmit+0x6f4/0x73c [mac80211]
[  676.030000] [<8014361c>] pktgen_thread_worker+0xfac/0x16f8
[  676.030000] [<8002ebe8>] kthread+0x7c/0x88
[  676.030000] [<80008e0c>] kernel_thread_helper+0x10/0x18
[  676.030000] 
[  676.030000] 
[  676.030000] Code: 24020001  10620005  2502001f <0200000d> 0804917a  00000000  2502001f  00441023  00531021 

Fix this by making a local copy of shared skbs prior to mangeling them.
To avoid copying the skb unnecessarily move the skb_copy call below the
checks that don't need write access to the skb.

Also, move the assignment of nh_pos and h_pos below the skb_copy to point
to the correct skb.

It would be possible to avoid another resize of the copied skb by using
skb_copy_expand instead of skb_copy but that would make the patch more
complex. Also, shared skbs are a corner case right now, so the resize
shouldn't matter much.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
---
 net/mac80211/tx.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 41bfbbf..99fa95b 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1741,15 +1741,13 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 	int nh_pos, h_pos;
 	struct sta_info *sta = NULL;
 	u32 sta_flags = 0;
+	struct sk_buff *tmp_skb;
 
 	if (unlikely(skb->len < ETH_HLEN)) {
 		ret = NETDEV_TX_OK;
 		goto fail;
 	}
 
-	nh_pos = skb_network_header(skb) - skb->data;
-	h_pos = skb_transport_header(skb) - skb->data;
-
 	/* convert Ethernet header to proper 802.11 header (based on
 	 * operation mode) */
 	ethertype = (skb->data[12] << 8) | skb->data[13];
@@ -1922,6 +1920,20 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		goto fail;
 	}
 
+	/*
+	 * If the skb is shared we need to obtain our own copy.
+	 */
+	if (skb_shared(skb)) {
+		tmp_skb = skb;
+		skb = skb_copy(skb, GFP_ATOMIC);
+		kfree_skb(tmp_skb);
+
+		if (!skb) {
+			ret = NETDEV_TX_OK;
+			goto fail;
+		}
+	}
+
 	hdr.frame_control = fc;
 	hdr.duration_id = 0;
 	hdr.seq_ctrl = 0;
@@ -1940,6 +1952,9 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
 		encaps_len = 0;
 	}
 
+	nh_pos = skb_network_header(skb) - skb->data;
+	h_pos = skb_transport_header(skb) - skb->data;
+
 	skb_pull(skb, skip_header_bytes);
 	nh_pos -= skip_header_bytes;
 	h_pos -= skip_header_bytes;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
  2010-12-02 17:44 [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs Helmut Schaa
@ 2010-12-02 17:46 ` Johannes Berg
  2010-12-02 17:58   ` Helmut Schaa
  2010-12-07 19:49 ` John W. Linville
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2010-12-02 17:46 UTC (permalink / raw)
  To: Helmut Schaa; +Cc: John W. Linville, linux-wireless

On Thu, 2010-12-02 at 18:44 +0100, Helmut Schaa wrote:
> mac80211 doesn't handle shared skbs correctly at the moment. As a result
> a possible resize can trigger a BUG in pskb_expand_head.

Could monitor_start_xmit have the same issue?

johannes


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
  2010-12-02 17:46 ` Johannes Berg
@ 2010-12-02 17:58   ` Helmut Schaa
  0 siblings, 0 replies; 6+ messages in thread
From: Helmut Schaa @ 2010-12-02 17:58 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless

Am Donnerstag 02 Dezember 2010 schrieb Johannes Berg:
> On Thu, 2010-12-02 at 18:44 +0100, Helmut Schaa wrote:
> > mac80211 doesn't handle shared skbs correctly at the moment. As a result
> > a possible resize can trigger a BUG in pskb_expand_head.
> 
> Could monitor_start_xmit have the same issue?

Hmm, good point. Are we expecting shared skbs on monitor interfaces somehow?

To be honest I don't know in which situations shared skbs are used (despite
the pktgen case I've run into) and thus cannot make a qualified statement :)

pktgen only generates 802.3 frames AFAICS and not arbitrary frames, so this
is not usable for monitor interfaces (yet?).

Helmut

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
  2010-12-02 17:44 [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs Helmut Schaa
  2010-12-02 17:46 ` Johannes Berg
@ 2010-12-07 19:49 ` John W. Linville
  2010-12-07 20:04   ` Johannes Berg
  2010-12-07 20:39   ` Helmut Schaa
  1 sibling, 2 replies; 6+ messages in thread
From: John W. Linville @ 2010-12-07 19:49 UTC (permalink / raw)
  To: Helmut Schaa; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 02, 2010 at 06:44:09PM +0100, Helmut Schaa wrote:
> mac80211 doesn't handle shared skbs correctly at the moment. As a result
> a possible resize can trigger a BUG in pskb_expand_head.
> 
> [  676.030000] Kernel bug detected[#1]:
> [  676.030000] Cpu 0
> [  676.030000] $ 0   : 00000000 00000000 819662ff 00000002
> [  676.030000] $ 4   : 81966200 00000020 00000000 00000020
> [  676.030000] $ 8   : 819662e0 800043c0 00000002 00020000
> [  676.030000] $12   : 3b9aca00 00000000 00000000 00470000
> [  676.030000] $16   : 80ea2000 00000000 00000000 00000000
> [  676.030000] $20   : 818aa200 80ea2018 80ea2000 00000008
> [  676.030000] $24   : 00000002 800ace5c                  
> [  676.030000] $28   : 8199a000 8199bd20 81938f88 80f180d4
> [  676.030000] Hi    : 0000026e
> [  676.030000] Lo    : 0000757e
> [  676.030000] epc   : 801245e4 pskb_expand_head+0x44/0x1d8
> [  676.030000]     Not tainted
> [  676.030000] ra    : 80f180d4 ieee80211_skb_resize+0xb0/0x114 [mac80211]
> [  676.030000] Status: 1000a403    KERNEL EXL IE 
> [  676.030000] Cause : 10800024
> [  676.030000] PrId  : 0001964c (MIPS 24Kc)
> [  676.030000] Modules linked in: mac80211_hwsim rt2800lib rt2x00soc rt2x00pci rt2x00lib mac80211 crc_itu_t crc_ccitt cfg80211 compat arc4 aes_generic deflate ecb cbc [last unloaded: rt2800pci]
> [  676.030000] Process kpktgend_0 (pid: 97, threadinfo=8199a000, task=81879f48, tls=00000000)
> [  676.030000] Stack : ffffffff 00000000 00000000 00000014 00000004 80ea2000 00000000 00000000
> [  676.030000]         818aa200 80f180d4 ffffffff 0000000a 81879f78 81879f48 81879f48 00000018
> [  676.030000]         81966246 80ea2000 818432e0 80f1a420 80203050 81814d98 00000001 81879f48
> [  676.030000]         81879f48 00000018 81966246 818432e0 0000001a 8199bdd4 0000001c 80f1b72c
> [  676.030000]         80203020 8001292c 80ef4aa2 7f10b55d 801ab5b8 81879f48 00000188 80005c90
> [  676.030000]         ...
> [  676.030000] Call Trace:
> [  676.030000] [<801245e4>] pskb_expand_head+0x44/0x1d8
> [  676.030000] [<80f180d4>] ieee80211_skb_resize+0xb0/0x114 [mac80211]
> [  676.030000] [<80f1a420>] ieee80211_xmit+0x150/0x22c [mac80211]
> [  676.030000] [<80f1b72c>] ieee80211_subif_start_xmit+0x6f4/0x73c [mac80211]
> [  676.030000] [<8014361c>] pktgen_thread_worker+0xfac/0x16f8
> [  676.030000] [<8002ebe8>] kthread+0x7c/0x88
> [  676.030000] [<80008e0c>] kernel_thread_helper+0x10/0x18
> [  676.030000] 
> [  676.030000] 
> [  676.030000] Code: 24020001  10620005  2502001f <0200000d> 0804917a  00000000  2502001f  00441023  00531021 
> 
> Fix this by making a local copy of shared skbs prior to mangeling them.
> To avoid copying the skb unnecessarily move the skb_copy call below the
> checks that don't need write access to the skb.
> 
> Also, move the assignment of nh_pos and h_pos below the skb_copy to point
> to the correct skb.
> 
> It would be possible to avoid another resize of the copied skb by using
> skb_copy_expand instead of skb_copy but that would make the patch more
> complex. Also, shared skbs are a corner case right now, so the resize
> shouldn't matter much.
> 
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>

Is this intended for 2.6.37?  It looks like it would apply there.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
  2010-12-07 19:49 ` John W. Linville
@ 2010-12-07 20:04   ` Johannes Berg
  2010-12-07 20:39   ` Helmut Schaa
  1 sibling, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2010-12-07 20:04 UTC (permalink / raw)
  To: John W. Linville; +Cc: Helmut Schaa, linux-wireless

On Tue, 2010-12-07 at 14:49 -0500, John W. Linville wrote:

> > It would be possible to avoid another resize of the copied skb by using
> > skb_copy_expand instead of skb_copy but that would make the patch more
> > complex. Also, shared skbs are a corner case right now, so the resize
> > shouldn't matter much.
> > 
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
> 
> Is this intended for 2.6.37?  It looks like it would apply there.

Yeah, and probably older stable too.

johannes


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs
  2010-12-07 19:49 ` John W. Linville
  2010-12-07 20:04   ` Johannes Berg
@ 2010-12-07 20:39   ` Helmut Schaa
  1 sibling, 0 replies; 6+ messages in thread
From: Helmut Schaa @ 2010-12-07 20:39 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Johannes Berg

Am Dienstag, 7. Dezember 2010 schrieb John W. Linville:
> On Thu, Dec 02, 2010 at 06:44:09PM +0100, Helmut Schaa wrote:
> > mac80211 doesn't handle shared skbs correctly at the moment. As a result
> > a possible resize can trigger a BUG in pskb_expand_head.
> > 
> > [  676.030000] Kernel bug detected[#1]:
> > [  676.030000] Cpu 0
> > [  676.030000] $ 0   : 00000000 00000000 819662ff 00000002
> > [  676.030000] $ 4   : 81966200 00000020 00000000 00000020
> > [  676.030000] $ 8   : 819662e0 800043c0 00000002 00020000
> > [  676.030000] $12   : 3b9aca00 00000000 00000000 00470000
> > [  676.030000] $16   : 80ea2000 00000000 00000000 00000000
> > [  676.030000] $20   : 818aa200 80ea2018 80ea2000 00000008
> > [  676.030000] $24   : 00000002 800ace5c                  
> > [  676.030000] $28   : 8199a000 8199bd20 81938f88 80f180d4
> > [  676.030000] Hi    : 0000026e
> > [  676.030000] Lo    : 0000757e
> > [  676.030000] epc   : 801245e4 pskb_expand_head+0x44/0x1d8
> > [  676.030000]     Not tainted
> > [  676.030000] ra    : 80f180d4 ieee80211_skb_resize+0xb0/0x114 [mac80211]
> > [  676.030000] Status: 1000a403    KERNEL EXL IE 
> > [  676.030000] Cause : 10800024
> > [  676.030000] PrId  : 0001964c (MIPS 24Kc)
> > [  676.030000] Modules linked in: mac80211_hwsim rt2800lib rt2x00soc rt2x00pci rt2x00lib mac80211 crc_itu_t crc_ccitt cfg80211 compat arc4 aes_generic deflate ecb cbc [last unloaded: rt2800pci]
> > [  676.030000] Process kpktgend_0 (pid: 97, threadinfo=8199a000, task=81879f48, tls=00000000)
> > [  676.030000] Stack : ffffffff 00000000 00000000 00000014 00000004 80ea2000 00000000 00000000
> > [  676.030000]         818aa200 80f180d4 ffffffff 0000000a 81879f78 81879f48 81879f48 00000018
> > [  676.030000]         81966246 80ea2000 818432e0 80f1a420 80203050 81814d98 00000001 81879f48
> > [  676.030000]         81879f48 00000018 81966246 818432e0 0000001a 8199bdd4 0000001c 80f1b72c
> > [  676.030000]         80203020 8001292c 80ef4aa2 7f10b55d 801ab5b8 81879f48 00000188 80005c90
> > [  676.030000]         ...
> > [  676.030000] Call Trace:
> > [  676.030000] [<801245e4>] pskb_expand_head+0x44/0x1d8
> > [  676.030000] [<80f180d4>] ieee80211_skb_resize+0xb0/0x114 [mac80211]
> > [  676.030000] [<80f1a420>] ieee80211_xmit+0x150/0x22c [mac80211]
> > [  676.030000] [<80f1b72c>] ieee80211_subif_start_xmit+0x6f4/0x73c [mac80211]
> > [  676.030000] [<8014361c>] pktgen_thread_worker+0xfac/0x16f8
> > [  676.030000] [<8002ebe8>] kthread+0x7c/0x88
> > [  676.030000] [<80008e0c>] kernel_thread_helper+0x10/0x18
> > [  676.030000] 
> > [  676.030000] 
> > [  676.030000] Code: 24020001  10620005  2502001f <0200000d> 0804917a  00000000  2502001f  00441023  00531021 
> > 
> > Fix this by making a local copy of shared skbs prior to mangeling them.
> > To avoid copying the skb unnecessarily move the skb_copy call below the
> > checks that don't need write access to the skb.
> > 
> > Also, move the assignment of nh_pos and h_pos below the skb_copy to point
> > to the correct skb.
> > 
> > It would be possible to avoid another resize of the copied skb by using
> > skb_copy_expand instead of skb_copy but that would make the patch more
> > complex. Also, shared skbs are a corner case right now, so the resize
> > shouldn't matter much.
> > 
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
> 
> Is this intended for 2.6.37?  It looks like it would apply there.

Fine with me, however, the patch is based on wireless-testing.

Thanks,
Helmut


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-12-07 20:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-02 17:44 [PATCH] mac80211: Fix BUG in pskb_expand_head when transmitting shared skbs Helmut Schaa
2010-12-02 17:46 ` Johannes Berg
2010-12-02 17:58   ` Helmut Schaa
2010-12-07 19:49 ` John W. Linville
2010-12-07 20:04   ` Johannes Berg
2010-12-07 20:39   ` Helmut Schaa

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).