2016-09-29 19:40:56

by Jes Sorensen

[permalink] [raw]
Subject: [PATCH 0/2] rtl8xxxu: Fix memory leak and big endian bug

From: Jes Sorensen <[email protected]>

Hi,

These two patches for 4.9 fix a potential memory leak for gen1 parts
and a big endian problem reporting mactime.

While I have only seen it trigger for 8188eu devices, in code which
has not yet been submitted, it could potentially hit other parts such
as 8723au, 8188cu, 8192cu. I prefer to get it fixed asap.

The second bug is a double byte-swap of the mactime read from the RX
packet descriptor.

Both fixes should be applicable to stable-4.8.

Cheers,
Jes


Jes Sorensen (2):
rtl8xxxu: Fix memory leak in handling rxdesc16 packets
rtl8xxxu: Fix big-endian problem reporting mactime

drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 ++--
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++++---
2 files changed, 10 insertions(+), 5 deletions(-)

--
2.7.4


2016-09-29 19:40:57

by Jes Sorensen

[permalink] [raw]
Subject: [PATCH 2/2] rtl8xxxu: Fix big-endian problem reporting mactime

From: Jes Sorensen <[email protected]>

The full RX descriptor is converted so converting tsfl again would
return it to it's original endian value.

Signed-off-by: Jes Sorensen <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 ++--
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 10166289..08d587a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -238,7 +238,7 @@ struct rtl8xxxu_rxdesc16 {
u32 pattern1match:1;
u32 pattern0match:1;
#endif
- __le32 tsfl;
+ u32 tsfl;
#if 0
u32 bassn:12;
u32 bavld:1;
@@ -368,7 +368,7 @@ struct rtl8xxxu_rxdesc24 {
u32 ldcp:1;
u32 splcp:1;
#endif
- __le32 tsfl;
+ u32 tsfl;
};

struct rtl8xxxu_txdesc32 {
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index a96ff17..a5e6ec2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5220,7 +5220,7 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
rx_desc->rxmcs);

- rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+ rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;

if (!rx_desc->swdec)
@@ -5290,7 +5290,7 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
rtl8xxxu_rx_parse_phystats(priv, rx_status, phy_stats,
rx_desc->rxmcs);

- rx_status->mactime = le32_to_cpu(rx_desc->tsfl);
+ rx_status->mactime = rx_desc->tsfl;
rx_status->flag |= RX_FLAG_MACTIME_START;

if (!rx_desc->swdec)
--
2.7.4

2016-09-29 19:40:57

by Jes Sorensen

[permalink] [raw]
Subject: [PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

From: Jes Sorensen <[email protected]>

A device running without RX package aggregation could return more data
in the USB packet than the actual network packet. In this case the
could would clone the skb but then determine that that there was no
packet to handle and exit without freeing the cloned skb first.

This has so far only been observed with 8188eu devices, but could
affect others.

Signed-off-by: Jes Sorensen <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index b2d7f6e..a96ff17 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5197,7 +5197,12 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
pkt_offset = roundup(pkt_len + drvinfo_sz + desc_shift +
sizeof(struct rtl8xxxu_rxdesc16), 128);

- if (pkt_cnt > 1)
+ /*
+ * Only clone the skb if there's enough data at the end to
+ * at least cover the rx descriptor
+ */
+ if (pkt_cnt > 1 &&
+ urb_len > (pkt_offset + sizeof(struct rtl8xxxu_rxdesc16)))
next_skb = skb_clone(skb, GFP_ATOMIC);

rx_status = IEEE80211_SKB_RXCB(skb);
--
2.7.4

2016-10-03 07:24:26

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

[email protected] writes:

> From: Jes Sorensen <[email protected]>
>
> A device running without RX package aggregation could return more data
> in the USB packet than the actual network packet. In this case the
> could would clone the skb but then determine that that there was no
> packet to handle and exit without freeing the cloned skb first.
>
> This has so far only been observed with 8188eu devices, but could
> affect others.
>
> Signed-off-by: Jes Sorensen <[email protected]>

Before I commit I'll add:

Cc: [email protected] # 4.8+

--
Kalle Valo

2016-10-03 12:34:44

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

Kalle Valo <[email protected]> writes:
> [email protected] writes:
>
>> From: Jes Sorensen <[email protected]>
>>
>> A device running without RX package aggregation could return more data
>> in the USB packet than the actual network packet. In this case the
>> could would clone the skb but then determine that that there was no
>> packet to handle and exit without freeing the cloned skb first.
>
> s/case the/case we/? I can edit that before applying the patch.

Sounds good - thanks!

Jes

2016-10-07 11:22:58

by Kalle Valo

[permalink] [raw]
Subject: Re: [1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

Jes Sorensen <[email protected]> wrote:
> From: Jes Sorensen <[email protected]>
>
> A device running without RX package aggregation could return more data
> in the USB packet than the actual network packet. In this case we
> could would clone the skb but then determine that that there was no
> packet to handle and exit without freeing the cloned skb first.
>
> This has so far only been observed with 8188eu devices, but could
> affect others.
>
> Signed-off-by: Jes Sorensen <[email protected]>
> Cc: [email protected] # 4.8+

2 patches applied to wireless-drivers.git, thanks.

1e54134ccad0 rtl8xxxu: Fix memory leak in handling rxdesc16 packets
8a55698f2f29 rtl8xxxu: Fix big-endian problem reporting mactime

--
https://patchwork.kernel.org/patch/9356961/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2016-10-03 07:25:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtl8xxxu: Fix memory leak in handling rxdesc16 packets

[email protected] writes:

> From: Jes Sorensen <[email protected]>
>
> A device running without RX package aggregation could return more data
> in the USB packet than the actual network packet. In this case the
> could would clone the skb but then determine that that there was no
> packet to handle and exit without freeing the cloned skb first.

s/case the/case we/? I can edit that before applying the patch.

--
Kalle Valo

2016-10-03 12:34:49

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtl8xxxu: Fix big-endian problem reporting mactime

Kalle Valo <[email protected]> writes:
> [email protected] writes:
>
>> From: Jes Sorensen <[email protected]>
>>
>> The full RX descriptor is converted so converting tsfl again would
>> return it to it's original endian value.
>>
>> Signed-off-by: Jes Sorensen <[email protected]>
>
> I'll add:
>
> Cc: [email protected] # 4.8+

Appreciate it!

Jes

2016-10-03 07:26:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtl8xxxu: Fix big-endian problem reporting mactime

[email protected] writes:

> From: Jes Sorensen <[email protected]>
>
> The full RX descriptor is converted so converting tsfl again would
> return it to it's original endian value.
>
> Signed-off-by: Jes Sorensen <[email protected]>

I'll add:

Cc: [email protected] # 4.8+

--
Kalle Valo