Re: [PATCH] mac80211: Fix invalid 'info' access in xmit-fast.

[Ben Greear]

On 01/02/2017 02:24 AM, Johannes Berg wrote:

On Fri, 2016-12-30 at 10:49 -0800, gree...@candelatech.com wrote:

From: Ben Greear 

ieee80211_xmit_fast assigns info from the passed-in
skb, but then later it also checks for skb_shared(skb),
and may create a new skb based on that check.

But, the code did not re-assign 'info', so it pointed into
the old shared skb.  This leads to all sorts of problems,
most obviously crashes since the new skb's info->hw_queue is not
properly initialized.

Bug was seen by sending pktgen packets on a bridge that
had an AP network interface in it.  hw-queue was out of
range, which made it crash like this:

BUG: unable to handle kernel NULL pointer dereference
at   (null)
IP: [] ieee80211_tx_frags+0x232/0x4c0 [mac80211]
PGD 0
Oops: 0002 [#1] PREEMPT SMP
CPU: 0 PID: 1512 Comm: kpktgend_0 Tainted: G   O4.7.10+
#24
Hardware name: To be filled by O.E.M. To be filled by
O.E.M./ChiefRiver, BIOS 4.6.5 06/07/2013
task: 8802132f3a00 ti: 8800362ac000 task.ti: 8800362ac000
RIP: 0010:[]  []
ieee80211_tx_frags+0x232/0x4c0 [mac80211]
RSP: 0018:8800362afa28  EFLAGS: 00010086
RAX: 8802130a22c0 RBX: 8802130a13a0 RCX: 880035ef2200
RDX: 880035ef2200 RSI: 0292 RDI: 
RBP: 8800362afa90 R08:  R09: 
R10: 0088 R11: 0001 R12: 880035ef2200
R13: 880035dabb90 R14: 8800362afb18 R15: 0cc0
FS:  () GS:88021e20()
knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 01e06000 CR4: 001406f0
Stack:
  0292 880035daa880 8802130a0b20 8800d4f01808
  00ff8800362afaa0 8800362afb18 8802130a13c0 88021e20db68
  880035daa880 8800362afb18 880035ef2200 88020f6ca300
Call Trace:
  [] __ieee80211_subif_start_xmit+0xc13/0xc30
[mac80211]
  [] ? find_busiest_group+0x35/0x4a0
  [] ieee80211_subif_start_xmit+0xb/0x10 [mac80211]
  [] dev_hard_start_xmit+0x9b/0x220
  [] sch_direct_xmit+0xd6/0x1a0
  [] __dev_queue_xmit+0x3be/0x6c0
  [] ? finish_task_switch+0x73/0x1f0
  [] dev_queue_xmit+0xd/0x10
  [] br_dev_queue_push_xmit+0x75/0x140 [bridge]
  [] br_forward_finish+0x29/0xa0 [bridge]
  [] ? del_timer_sync+0x43/0x50
  [] __br_deliver+0x62/0x130 [bridge]
  [] ? __getnstimeofday64+0x37/0xd0
  [] ? getnstimeofday64+0x9/0x30
  [] br_deliver+0x56/0x60 [bridge]
  [] br_dev_xmit+0x1c2/0x250 [bridge]
  [] pktgen_thread_worker+0x174c/0x2471 [pktgen]
  [] ? __schedule+0x30a/0x7c0
  [] ? wake_atomic_t_function+0x60/0x60
  [] ? pktgen_rem_all_ifs+0x80/0x80 [pktgen]
  [] kthread+0xc4/0xe0
  [] ret_from_fork+0x1f/0x40
  [] ? kthread_worker_fn+0x180/0x180

Fix this by re-assigning info after creating new one.

Signed-off-by: Ben Greear 
---

This patch is against 4.7.10+


And thus didn't apply - I've replaced it with my own patch which I'll
send out in a moment


Thanks for your patch...  It is cleaner than what I originally wrote,
and I was out the door before I thought to re-write it as you did.

FYI, the same patch applies as far back as 4.4...I didn't look at anything 
older.

Ben



johannes




--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH] mac80211: Fix invalid 'info' access in xmit-fast.

[Johannes Berg]

On Fri, 2016-12-30 at 10:49 -0800, gree...@candelatech.com wrote:
> From: Ben Greear 
> 
> ieee80211_xmit_fast assigns info from the passed-in
> skb, but then later it also checks for skb_shared(skb),
> and may create a new skb based on that check.
> 
> But, the code did not re-assign 'info', so it pointed into
> the old shared skb.  This leads to all sorts of problems,
> most obviously crashes since the new skb's info->hw_queue is not
> properly initialized.
> 
> Bug was seen by sending pktgen packets on a bridge that
> had an AP network interface in it.  hw-queue was out of
> range, which made it crash like this:
> 
> BUG: unable to handle kernel NULL pointer dereference
> at   (null)
> IP: [] ieee80211_tx_frags+0x232/0x4c0 [mac80211]
> PGD 0
> Oops: 0002 [#1] PREEMPT SMP
> CPU: 0 PID: 1512 Comm: kpktgend_0 Tainted: G   O4.7.10+
> #24
> Hardware name: To be filled by O.E.M. To be filled by
> O.E.M./ChiefRiver, BIOS 4.6.5 06/07/2013
> task: 8802132f3a00 ti: 8800362ac000 task.ti: 8800362ac000
> RIP: 0010:[]  []
> ieee80211_tx_frags+0x232/0x4c0 [mac80211]
> RSP: 0018:8800362afa28  EFLAGS: 00010086
> RAX: 8802130a22c0 RBX: 8802130a13a0 RCX: 880035ef2200
> RDX: 880035ef2200 RSI: 0292 RDI: 
> RBP: 8800362afa90 R08:  R09: 
> R10: 0088 R11: 0001 R12: 880035ef2200
> R13: 880035dabb90 R14: 8800362afb18 R15: 0cc0
> FS:  () GS:88021e20()
> knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2:  CR3: 01e06000 CR4: 001406f0
> Stack:
>  0292 880035daa880 8802130a0b20 8800d4f01808
>  00ff8800362afaa0 8800362afb18 8802130a13c0 88021e20db68
>  880035daa880 8800362afb18 880035ef2200 88020f6ca300
> Call Trace:
>  [] __ieee80211_subif_start_xmit+0xc13/0xc30
> [mac80211]
>  [] ? find_busiest_group+0x35/0x4a0
>  [] ieee80211_subif_start_xmit+0xb/0x10 [mac80211]
>  [] dev_hard_start_xmit+0x9b/0x220
>  [] sch_direct_xmit+0xd6/0x1a0
>  [] __dev_queue_xmit+0x3be/0x6c0
>  [] ? finish_task_switch+0x73/0x1f0
>  [] dev_queue_xmit+0xd/0x10
>  [] br_dev_queue_push_xmit+0x75/0x140 [bridge]
>  [] br_forward_finish+0x29/0xa0 [bridge]
>  [] ? del_timer_sync+0x43/0x50
>  [] __br_deliver+0x62/0x130 [bridge]
>  [] ? __getnstimeofday64+0x37/0xd0
>  [] ? getnstimeofday64+0x9/0x30
>  [] br_deliver+0x56/0x60 [bridge]
>  [] br_dev_xmit+0x1c2/0x250 [bridge]
>  [] pktgen_thread_worker+0x174c/0x2471 [pktgen]
>  [] ? __schedule+0x30a/0x7c0
>  [] ? wake_atomic_t_function+0x60/0x60
>  [] ? pktgen_rem_all_ifs+0x80/0x80 [pktgen]
>  [] kthread+0xc4/0xe0
>  [] ret_from_fork+0x1f/0x40
>  [] ? kthread_worker_fn+0x180/0x180
> 
> Fix this by re-assigning info after creating new one.
> 
> Signed-off-by: Ben Greear 
> ---
> 
> This patch is against 4.7.10+

And thus didn't apply - I've replaced it with my own patch which I'll
send out in a moment

johannes

[PATCH] mac80211: Fix invalid 'info' access in xmit-fast.

[greearb]

From: Ben Greear 

ieee80211_xmit_fast assigns info from the passed-in
skb, but then later it also checks for skb_shared(skb),
and may create a new skb based on that check.

But, the code did not re-assign 'info', so it pointed into
the old shared skb.  This leads to all sorts of problems,
most obviously crashes since the new skb's info->hw_queue is not
properly initialized.

Bug was seen by sending pktgen packets on a bridge that
had an AP network interface in it.  hw-queue was out of
range, which made it crash like this:

BUG: unable to handle kernel NULL pointer dereference at   (null)
IP: [] ieee80211_tx_frags+0x232/0x4c0 [mac80211]
PGD 0
Oops: 0002 [#1] PREEMPT SMP
CPU: 0 PID: 1512 Comm: kpktgend_0 Tainted: G   O4.7.10+ #24
Hardware name: To be filled by O.E.M. To be filled by O.E.M./ChiefRiver, BIOS 
4.6.5 06/07/2013
task: 8802132f3a00 ti: 8800362ac000 task.ti: 8800362ac000
RIP: 0010:[]  [] 
ieee80211_tx_frags+0x232/0x4c0 [mac80211]
RSP: 0018:8800362afa28  EFLAGS: 00010086
RAX: 8802130a22c0 RBX: 8802130a13a0 RCX: 880035ef2200
RDX: 880035ef2200 RSI: 0292 RDI: 
RBP: 8800362afa90 R08:  R09: 
R10: 0088 R11: 0001 R12: 880035ef2200
R13: 880035dabb90 R14: 8800362afb18 R15: 0cc0
FS:  () GS:88021e20() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 01e06000 CR4: 001406f0
Stack:
 0292 880035daa880 8802130a0b20 8800d4f01808
 00ff8800362afaa0 8800362afb18 8802130a13c0 88021e20db68
 880035daa880 8800362afb18 880035ef2200 88020f6ca300
Call Trace:
 [] __ieee80211_subif_start_xmit+0xc13/0xc30 [mac80211]
 [] ? find_busiest_group+0x35/0x4a0
 [] ieee80211_subif_start_xmit+0xb/0x10 [mac80211]
 [] dev_hard_start_xmit+0x9b/0x220
 [] sch_direct_xmit+0xd6/0x1a0
 [] __dev_queue_xmit+0x3be/0x6c0
 [] ? finish_task_switch+0x73/0x1f0
 [] dev_queue_xmit+0xd/0x10
 [] br_dev_queue_push_xmit+0x75/0x140 [bridge]
 [] br_forward_finish+0x29/0xa0 [bridge]
 [] ? del_timer_sync+0x43/0x50
 [] __br_deliver+0x62/0x130 [bridge]
 [] ? __getnstimeofday64+0x37/0xd0
 [] ? getnstimeofday64+0x9/0x30
 [] br_deliver+0x56/0x60 [bridge]
 [] br_dev_xmit+0x1c2/0x250 [bridge]
 [] pktgen_thread_worker+0x174c/0x2471 [pktgen]
 [] ? __schedule+0x30a/0x7c0
 [] ? wake_atomic_t_function+0x60/0x60
 [] ? pktgen_rem_all_ifs+0x80/0x80 [pktgen]
 [] kthread+0xc4/0xe0
 [] ret_from_fork+0x1f/0x40
 [] ? kthread_worker_fn+0x180/0x180

Fix this by re-assigning info after creating new one.

Signed-off-by: Ben Greear 
---

This patch is against 4.7.10+

 net/mac80211/tx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 0573ab9..3da5f94 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3081,6 +3081,7 @@ static bool ieee80211_xmit_fast(struct 
ieee80211_sub_if_data *sdata,
 
if (!skb)
return true;
+   info = IEEE80211_SKB_CB(skb);
}
 
ieee80211_tx_stats(dev, skb->len + extra_head);
-- 
2.4.11