diff mbox series

wifi: rtl8xxxu: Fix memory leaks with RTL8723BU, RTL8192EU

Message ID 03b099c1-c671-d252-36f4-57b70d721f9d@gmail.com (mailing list archive)
State Accepted
Commit b39f662ce1648db0b9de32e6a849b098480793cb
Delegated to: Kalle Valo
Headers show
Series wifi: rtl8xxxu: Fix memory leaks with RTL8723BU, RTL8192EU | expand

Commit Message

Bitterblue Smith Dec. 22, 2022, 11:48 a.m. UTC
The wifi + bluetooth combo chip RTL8723BU can leak memory (especially?)
when it's connected to a bluetooth audio device. The busy bluetooth
traffic generates lots of C2H (card to host) messages, which are not
freed correctly.

To fix this, move the dev_kfree_skb() call in rtl8xxxu_c2hcmd_callback()
inside the loop where skb_dequeue() is called.

The RTL8192EU leaks memory because the C2H messages are added to the
queue and left there forever. (This was fine in the past because it
probably wasn't sending any C2H messages until commit e542e66b7c2e
("wifi: rtl8xxxu: gen2: Turn on the rate control"). Since that commit
it sends a C2H message when the TX rate changes.)

To fix this, delete the check for rf_paths > 1 and the goto. Let the
function process the C2H messages from RTL8192EU like the ones from
the other chips.

Theoretically the RTL8188FU could also leak like RTL8723BU, but it
most likely doesn't send C2H messages frequently enough.

This change was tested with RTL8723BU by Erhard F. I tested it with
RTL8188FU and RTL8192EU.

Reported-by: Erhard F. <erhard_f@mailbox.org>
Tested-by: Erhard F. <erhard_f@mailbox.org>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215197
Fixes: e542e66b7c2e ("rtl8xxxu: add bluetooth co-existence support for single antenna")
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Ping-Ke Shih Dec. 23, 2022, 1:54 a.m. UTC | #1
> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Thursday, December 22, 2022 7:48 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>
> Subject: [PATCH] wifi: rtl8xxxu: Fix memory leaks with RTL8723BU, RTL8192EU
> 
> The wifi + bluetooth combo chip RTL8723BU can leak memory (especially?)
> when it's connected to a bluetooth audio device. The busy bluetooth
> traffic generates lots of C2H (card to host) messages, which are not
> freed correctly.
> 
> To fix this, move the dev_kfree_skb() call in rtl8xxxu_c2hcmd_callback()
> inside the loop where skb_dequeue() is called.
> 
> The RTL8192EU leaks memory because the C2H messages are added to the
> queue and left there forever. (This was fine in the past because it
> probably wasn't sending any C2H messages until commit e542e66b7c2e
> ("wifi: rtl8xxxu: gen2: Turn on the rate control"). Since that commit
> it sends a C2H message when the TX rate changes.)
> 
> To fix this, delete the check for rf_paths > 1 and the goto. Let the
> function process the C2H messages from RTL8192EU like the ones from
> the other chips.
> 
> Theoretically the RTL8188FU could also leak like RTL8723BU, but it
> most likely doesn't send C2H messages frequently enough.
> 
> This change was tested with RTL8723BU by Erhard F. I tested it with
> RTL8188FU and RTL8192EU.
> 
> Reported-by: Erhard F. <erhard_f@mailbox.org>
> Tested-by: Erhard F. <erhard_f@mailbox.org>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215197
> Fixes: e542e66b7c2e ("rtl8xxxu: add bluetooth co-existence support for single antenna")
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index fd97c040948a..03ffb99da7e2 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5702,9 +5702,6 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
>  	btcoex = &priv->bt_coex;
>  	rarpt = &priv->ra_report;
> 
> -	if (priv->rf_paths > 1)
> -		goto out;
> -
>  	while (!skb_queue_empty(&priv->c2hcmd_queue)) {
>  		skb = skb_dequeue(&priv->c2hcmd_queue);
> 
> @@ -5737,10 +5734,9 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
>  		default:
>  			break;
>  		}
> -	}
> 
> -out:
> -	dev_kfree_skb(skb);
> +		dev_kfree_skb(skb);
> +	}
>  }
> 
>  static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,
> --
> 2.38.0
> 
> ------Please consider the environment before printing this e-mail.
Kalle Valo Jan. 16, 2023, 4:21 p.m. UTC | #2
Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:

> The wifi + bluetooth combo chip RTL8723BU can leak memory (especially?)
> when it's connected to a bluetooth audio device. The busy bluetooth
> traffic generates lots of C2H (card to host) messages, which are not
> freed correctly.
> 
> To fix this, move the dev_kfree_skb() call in rtl8xxxu_c2hcmd_callback()
> inside the loop where skb_dequeue() is called.
> 
> The RTL8192EU leaks memory because the C2H messages are added to the
> queue and left there forever. (This was fine in the past because it
> probably wasn't sending any C2H messages until commit e542e66b7c2e
> ("wifi: rtl8xxxu: gen2: Turn on the rate control"). Since that commit
> it sends a C2H message when the TX rate changes.)
> 
> To fix this, delete the check for rf_paths > 1 and the goto. Let the
> function process the C2H messages from RTL8192EU like the ones from
> the other chips.
> 
> Theoretically the RTL8188FU could also leak like RTL8723BU, but it
> most likely doesn't send C2H messages frequently enough.
> 
> This change was tested with RTL8723BU by Erhard F. I tested it with
> RTL8188FU and RTL8192EU.
> 
> Reported-by: Erhard F. <erhard_f@mailbox.org>
> Tested-by: Erhard F. <erhard_f@mailbox.org>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215197
> Fixes: e542e66b7c2e ("rtl8xxxu: add bluetooth co-existence support for single antenna")
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

Patch applied to wireless-next.git, thanks.

b39f662ce164 wifi: rtl8xxxu: Fix memory leaks with RTL8723BU, RTL8192EU
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index fd97c040948a..03ffb99da7e2 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5702,9 +5702,6 @@  static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
 	btcoex = &priv->bt_coex;
 	rarpt = &priv->ra_report;
 
-	if (priv->rf_paths > 1)
-		goto out;
-
 	while (!skb_queue_empty(&priv->c2hcmd_queue)) {
 		skb = skb_dequeue(&priv->c2hcmd_queue);
 
@@ -5737,10 +5734,9 @@  static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
 		default:
 			break;
 		}
-	}
 
-out:
-	dev_kfree_skb(skb);
+		dev_kfree_skb(skb);
+	}
 }
 
 static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,