[PATCH 0/3] rtl8192cu: Fixes for leaks and deadlock

[PATCH 0/3] rtl8192cu: Fixes for leaks and deadlock

[Reto Schneider]

Issues have been found on a Realtek RTL8188CUS/Atmel AT91SAM9G25
(ARMv5) machine. Testing has been done on the same.

Reto Schneider (3):
  rtl8192cu: Fix deadlock
  rtl8192cu: Prevent leaking urb
  rtl8192cu: Free ieee80211_hw if probing fails

 drivers/net/wireless/realtek/rtlwifi/usb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
2.20.1

[PATCH 1/3] rtl8192cu: Fix deadlock

[Reto Schneider]

Prevent code from calling itself indirectly, causing the driver to hang
and consume 100% CPU.

Without this fix, the following script can bring down a single CPU
system:
```
while true; do
  rmmod rtl8192cu
  modprobe rtl8192cu
done
```

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
---
 drivers/net/wireless/realtek/rtlwifi/usb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index c66c6dc00378..d3bbfc0171f6 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -680,8 +680,10 @@ static void _rtl_usb_cleanup_rx(struct ieee80211_hw *hw)
 	tasklet_kill(&rtlusb->rx_work_tasklet);
 	cancel_work_sync(&rtlpriv->works.lps_change_work);
 
-	flush_workqueue(rtlpriv->works.rtl_wq);
-	destroy_workqueue(rtlpriv->works.rtl_wq);
+	if (rtlpriv->works.rtl_wq) {
+		destroy_workqueue(rtlpriv->works.rtl_wq);
+		rtlpriv->works.rtl_wq = NULL;
+	}
 
 	skb_queue_purge(&rtlusb->rx_queue);
 
-- 
2.20.1

[PATCH 2/3] rtl8192cu: Prevent leaking urb

[Reto Schneider]

If usb_submit_urb fails the allocated urb should be unanchored and
released.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
---
 drivers/net/wireless/realtek/rtlwifi/usb.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index d3bbfc0171f6..1a9fcb2697bf 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -720,8 +720,11 @@ static int _rtl_usb_receive(struct ieee80211_hw *hw)
 
 		usb_anchor_urb(urb, &rtlusb->rx_submitted);
 		err = usb_submit_urb(urb, GFP_KERNEL);
-		if (err)
+		if (err) {
+			usb_unanchor_urb(urb);
+			usb_free_urb(urb);
 			goto err_out;
+		}
 		usb_free_urb(urb);
 	}
 	return 0;
-- 
2.20.1

[PATCH 3/3] rtl8192cu: Free ieee80211_hw if probing fails

[Reto Schneider]

In case of an error, no one will use the allocated structure. Call
ieee80211_free_hw, same as in rtl_usb_disconnect.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
---
 drivers/net/wireless/realtek/rtlwifi/usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
index 1a9fcb2697bf..d05e709536ea 100644
--- a/drivers/net/wireless/realtek/rtlwifi/usb.c
+++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
@@ -1087,6 +1087,7 @@ int rtl_usb_probe(struct usb_interface *intf,
 	usb_put_dev(udev);
 	complete(&rtlpriv->firmware_loading_complete);
 	kfree(rtlpriv->usb_data);
+	ieee80211_free_hw(hw);
 	return -ENODEV;
 }
 EXPORT_SYMBOL(rtl_usb_probe);
-- 
2.20.1

Re: [PATCH 1/3] rtl8192cu: Fix deadlock

[Larry Finger]

On 6/22/20 8:21 AM, Reto Schneider wrote:
> Prevent code from calling itself indirectly, causing the driver to hang
> and consume 100% CPU.
> 
> Without this fix, the following script can bring down a single CPU
> system:
> ```
> while true; do
>    rmmod rtl8192cu
>    modprobe rtl8192cu
> done
> ```
> 
> Signed-off-by: Reto Schneider <code@reto-schneider.ch>

I have one small comment. Patches for the rtlwifi family of drivers have patch 
subjects of the form [PATCH] rtlwifi: rtl8192cu: blah blah. I'll let Kalle 
decide if he wants ignore my comment, manually change the subjects, or request a 
V2. Otherwise all 3 patches are OK.

ACKed-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

Re: [PATCH 1/3] rtl8192cu: Fix deadlock

[Kalle Valo]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 6/22/20 8:21 AM, Reto Schneider wrote:
>> Prevent code from calling itself indirectly, causing the driver to hang
>> and consume 100% CPU.
>>
>> Without this fix, the following script can bring down a single CPU
>> system:
>> ```
>> while true; do
>>    rmmod rtl8192cu
>>    modprobe rtl8192cu
>> done
>> ```
>>
>> Signed-off-by: Reto Schneider <code@reto-schneider.ch>
>
> I have one small comment. Patches for the rtlwifi family of drivers
> have patch subjects of the form [PATCH] rtlwifi: rtl8192cu: blah blah.
> I'll let Kalle decide if he wants ignore my comment, manually change
> the subjects, or request a V2. Otherwise all 3 patches are OK.

I can fix the titles during commit.

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

Re: [PATCH 1/3] rtlwifi: rtl8192cu: Fix deadlock

[Kalle Valo]

Reto Schneider <code@reto-schneider.ch> wrote:

> Prevent code from calling itself indirectly, causing the driver to hang
> and consume 100% CPU.
> 
> Without this fix, the following script can bring down a single CPU
> system:
> ```
> while true; do
>   rmmod rtl8192cu
>   modprobe rtl8192cu
> done
> ```
> 
> Signed-off-by: Reto Schneider <code@reto-schneider.ch>
> ACKed-by: Larry Finger <Larry.Finger@lwfinger.net>

3 patches applied to wireless-drivers-next.git, thanks.

ec89032cd148 rtlwifi: rtl8192cu: Fix deadlock
03128643eb54 rtlwifi: rtl8192cu: Prevent leaking urb
a7f7c15e945a rtlwifi: rtl8192cu: Free ieee80211_hw if probing fails

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

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