2009-03-23 16:32:36

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 6/8] mac80211: add skb length sanity checking

We just found a bug in zd1211rw where it would reject
packets in the ->tx() method but leave them modified,
which would cause retransmit attempts with completely
bogus skbs, eventually leading to a panic due to not
having enough headroom in those.

This patch adds a sanity check to mac80211 to catch
such driver mistakes; in this case we warn and drop
the skb.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/tx.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

--- wireless-testing.orig/net/mac80211/tx.c 2009-03-23 14:03:46.000000000 +0100
+++ wireless-testing/net/mac80211/tx.c 2009-03-23 14:03:47.000000000 +0100
@@ -1089,7 +1089,7 @@ static int __ieee80211_tx(struct ieee802
{
struct sk_buff *skb = *skbp, *next;
struct ieee80211_tx_info *info;
- int ret;
+ int ret, len;
bool fragm = false;

local->mdev->trans_start = jiffies;
@@ -1125,7 +1125,12 @@ static int __ieee80211_tx(struct ieee802
}

next = skb->next;
+ len = skb->len;
ret = local->ops->tx(local_to_hw(local), skb);
+ if (WARN_ON(ret != NETDEV_TX_OK && skb->len != len)) {
+ dev_kfree_skb(skb);
+ ret = NETDEV_TX_OK;
+ }
if (ret != NETDEV_TX_OK)
return IEEE80211_TX_AGAIN;
*skbp = skb = next;

--



2009-03-28 17:29:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 6/8] mac80211: add skb length sanity checking

On Fri, 2009-03-27 at 20:00 -0700, Luis R. Rodriguez wrote:

> >> This patch adds a sanity check to mac80211 to catch
> >> such driver mistakes; in this case we warn and drop
> >> the skb.
> >>
> >> Signed-off-by: Johannes Berg <[email protected]>
> >
> > Heh well this is obviously even a stable candidate and it got
> > merged already :D.
>
> Nevermind, I was looking at .30, heh. So if zd1211rw also manged the
> skb len in older kernels it seems this is stable candidate. Too lazy
> to check right now though.

The fixes should go to stable, but this won't apply without serious
backporting.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-28 03:00:36

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 6/8] mac80211: add skb length sanity checking

On Fri, Mar 27, 2009 at 7:40 PM, Luis R. Rodriguez
<[email protected]> wrote:
> On Mon, Mar 23, 2009 at 05:28:40PM +0100, Johannes Berg wrote:
>> We just found a bug in zd1211rw where it would reject
>> packets in the ->tx() method but leave them modified,
>> which would cause retransmit attempts with completely
>> bogus skbs, eventually leading to a panic due to not
>> having enough headroom in those.
>>
>> This patch adds a sanity check to mac80211 to catch
>> such driver mistakes; in this case we warn and drop
>> the skb.
>>
>> Signed-off-by: Johannes Berg <[email protected]>
>
> Heh well this is obviously even a stable candidate and it got
> merged already :D.

Nevermind, I was looking at .30, heh. So if zd1211rw also manged the
skb len in older kernels it seems this is stable candidate. Too lazy
to check right now though.

Luis

2009-03-28 02:40:27

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 6/8] mac80211: add skb length sanity checking

On Mon, Mar 23, 2009 at 05:28:40PM +0100, Johannes Berg wrote:
> We just found a bug in zd1211rw where it would reject
> packets in the ->tx() method but leave them modified,
> which would cause retransmit attempts with completely
> bogus skbs, eventually leading to a panic due to not
> having enough headroom in those.
>
> This patch adds a sanity check to mac80211 to catch
> such driver mistakes; in this case we warn and drop
> the skb.
>
> Signed-off-by: Johannes Berg <[email protected]>

Heh well this is obviously even a stable candidate and it got
merged already :D.

Luis