2011-04-07 18:05:40

by Jason Conti

[permalink] [raw]
Subject: [PATCH] p54: Initialize extra_len in p54_tx_80211

When compiling with gcc-4.5 and CONFIG_DEBUG_SECTION_MISMATCH=y (which enables
-fno-inline-functions-called-once), p54_tx_80211_header will not be
inlined and extra_len will quickly
become corrupted because it uninitialized. The outgoing buffer fills
up before an attempt
to associate with a WPA access point can complete (it goes into an
associating 1, 2, 3, timed out
direct probe 1, 2, 3 timed out loop with wpa_supplicant and the driver
constantly returns ENOMEM).

I think this was previously hidden because in gcc-4.4, as well a
gcc-4.5 without
-fno-inline-functions-called-once, p54_tx_80211_header is always
inlined, which may have limited
the corruption.

It was suggested I submit this upstream by Stefan Bader to resolve LP: #722185

This patch is against 2.6.38.2

Signed-off-by: Jason Conti
---
--- a/drivers/net/wireless/p54/txrx.c 2011-04-06 18:05:01.951581773 -0400
+++ b/drivers/net/wireless/p54/txrx.c 2011-04-06 18:05:26.195581762 -0400
@@ -705,7 +705,7 @@ int p54_tx_80211(struct ieee80211_hw *de
struct p54_tx_info *p54info;
struct p54_hdr *hdr;
struct p54_tx_data *txhdr;
- unsigned int padding, len, extra_len;
+ unsigned int padding, len, extra_len = 0;
int i, j, ridx;
u16 hdr_flags = 0, aid = 0;
u8 rate, queue = 0, crypt_offset = 0;


2011-04-07 19:06:23

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] p54: Initialize extra_len in p54_tx_80211

On 04/07/2011 01:05 PM, Jason Conti wrote:
> When compiling with gcc-4.5 and CONFIG_DEBUG_SECTION_MISMATCH=y (which enables
> -fno-inline-functions-called-once), p54_tx_80211_header will not be
> inlined and extra_len will quickly
> become corrupted because it uninitialized. The outgoing buffer fills
> up before an attempt
> to associate with a WPA access point can complete (it goes into an
> associating 1, 2, 3, timed out
> direct probe 1, 2, 3 timed out loop with wpa_supplicant and the driver
> constantly returns ENOMEM).
>
> I think this was previously hidden because in gcc-4.4, as well a
> gcc-4.5 without
> -fno-inline-functions-called-once, p54_tx_80211_header is always
> inlined, which may have limited
> the corruption.
>
> It was suggested I submit this upstream by Stefan Bader to resolve LP: #722185
>
> This patch is against 2.6.38.2
>
> Signed-off-by: Jason Conti
> ---
> --- a/drivers/net/wireless/p54/txrx.c 2011-04-06 18:05:01.951581773 -0400
> +++ b/drivers/net/wireless/p54/txrx.c 2011-04-06 18:05:26.195581762 -0400
> @@ -705,7 +705,7 @@ int p54_tx_80211(struct ieee80211_hw *de
> struct p54_tx_info *p54info;
> struct p54_hdr *hdr;
> struct p54_tx_data *txhdr;
> - unsigned int padding, len, extra_len;
> + unsigned int padding, len, extra_len = 0;
> int i, j, ridx;
> u16 hdr_flags = 0, aid = 0;
> u8 rate, queue = 0, crypt_offset = 0;
> --

ACK. Good catch.

Larry

2011-04-07 19:10:09

by Christian Lamparter

[permalink] [raw]
Subject: [PATCH -stable] p54: Initialize extra_len in p54_tx_80211

From: Jason Conti <[email protected]>

This patch fixes a very serious off-by-one bug in
the driver, which could leave the device in an
unresponsive state.

The problem was that the extra_len variable [used to
reserve extra scratch buffer space for the firmware]
was left uninitialized. Because p54_assign_address
later needs the value to reserve additional space,
the resulting frame could be to big for the small
device's memory window and everything would
immediately come to a grinding halt.

Reference: https://bugs.launchpad.net/bugs/722185

Cc: <[email protected]>
Acked-by: Christian Lamparter <[email protected]>
Signed-off-by: Jason Conti <[email protected]>
---
Janson Conti,

I hope you don't mind the "pretty-printing", right?
But, this is not a problem of gcc or debug options
and therefore I had to rewrite a few parts to make
it clear why this is a "-stable" patch.

Thanks for the patch, your work is highly appreciated!

Best Regards,
Christian
---
diff --git a/drivers/net/wireless/p54/txrx.c b/drivers/net/wireless/p54/txrx.c
index 3a93162..4d28b52 100644
--- a/drivers/net/wireless/p54/txrx.c
+++ b/drivers/net/wireless/p54/txrx.c
@@ -703,7 +703,7 @@ void p54_tx_80211(struct ieee80211_hw *dev, struct sk_buff *skb)
struct p54_tx_info *p54info;
struct p54_hdr *hdr;
struct p54_tx_data *txhdr;
- unsigned int padding, len, extra_len;
+ unsigned int padding, len, extra_len = 0;
int i, j, ridx;
u16 hdr_flags = 0, aid = 0;
u8 rate, queue = 0, crypt_offset = 0;