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

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

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

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

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

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

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 <Jes.Sorensen@redhat.com>
---
 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

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

[Kalle Valo]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> 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 <Jes.Sorensen@redhat.com>

Before I commit I'll add:

Cc: stable@vger.kernel.org # 4.8+

-- 
Kalle Valo

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

[Kalle Valo]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> 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

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

[Jes Sorensen]

Kalle Valo <kvalo@codeaurora.org> writes:
> Jes.Sorensen@redhat.com writes:
>
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> 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

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

[Kalle Valo]

Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> 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 <Jes.Sorensen@redhat.com>
> Cc: stable@vger.kernel.org # 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

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

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

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

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 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

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

[Kalle Valo]

Jes.Sorensen@redhat.com writes:

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

I'll add:

Cc: stable@vger.kernel.org # 4.8+

-- 
Kalle Valo

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

[Jes Sorensen]

Kalle Valo <kvalo@codeaurora.org> writes:
> Jes.Sorensen@redhat.com writes:
>
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> The full RX descriptor is converted so converting tsfl again would
>> return it to it's original endian value.
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> I'll add:
>
> Cc: stable@vger.kernel.org # 4.8+

Appreciate it!

Jes