[PATCH] wifi: mac80211: fix memory leak in ieee80211_register_hw()

[PATCH] wifi: mac80211: fix memory leak in ieee80211_register_hw()

[Zhengchao Shao]

When ieee80211_init_rate_ctrl_alg() failed in ieee80211_register_hw(),
it doesn't release local->fq. The memory leakage information is as
follows:
unreferenced object 0xffff888109cad400 (size 512):
  comm "insmod", pid 15145, jiffies 4295005736 (age 3670.100s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000d1eb4a9f>] __kmalloc+0x3e/0xb0
    [<00000000befc3e34>] ieee80211_txq_setup_flows+0x1fe/0xa10
    [<00000000b13f1457>] ieee80211_register_hw+0x1b64/0x3950
    [<00000000ba9f4e99>] 0xffffffffa02214db
    [<00000000833435c0>] 0xffffffffa024048d
    [<00000000a4ddd6ef>] do_one_initcall+0x10f/0x630
    [<0000000068f29e16>] do_init_module+0x19f/0x5e0
    [<00000000f52609b6>] load_module+0x64b7/0x6eb0
    [<00000000b628a5b3>] __do_sys_finit_module+0x140/0x200
    [<00000000c7f35d15>] do_syscall_64+0x35/0x80
    [<0000000044d8d0fd>] entry_SYSCALL_64_after_hwframe+0x46/0xb0

Fixes: a50e5fb8db83 ("mac80211: fix a kernel panic when TXing after TXQ teardown")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 net/mac80211/iface.c | 2 --
 net/mac80211/main.c  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 46f08ec5ed76..cec6dd577241 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -2326,8 +2326,6 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
 	WARN(local->open_count, "%s: open count remains %d\n",
 	     wiphy_name(local->hw.wiphy), local->open_count);
 
-	ieee80211_txq_teardown_flows(local);
-
 	mutex_lock(&local->iflist_mtx);
 	list_for_each_entry_safe(sdata, tmp, &local->interfaces, list) {
 		list_del(&sdata->list);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 02b5abc7326b..b6620c5fd9e0 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1435,6 +1435,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	ieee80211_remove_interfaces(local);
 	rtnl_unlock();
  fail_rate:
+	ieee80211_txq_teardown_flows(local);
  fail_flows:
 	ieee80211_led_exit(local);
 	destroy_workqueue(local->workqueue);
@@ -1469,6 +1470,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 	 * because the driver cannot be handing us frames any
 	 * more and the tasklet is killed.
 	 */
+	ieee80211_txq_teardown_flows(local);
 	ieee80211_remove_interfaces(local);
 
 	rtnl_unlock();
-- 
2.17.1

Re: [PATCH] wifi: mac80211: fix memory leak in ieee80211_register_hw()

[Johannes Berg]

On Tue, 2022-11-22 at 17:11 +0800, Zhengchao Shao wrote:
> 
> +++ b/net/mac80211/iface.c
> @@ -2326,8 +2326,6 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
>  	WARN(local->open_count, "%s: open count remains %d\n",
>  	     wiphy_name(local->hw.wiphy), local->open_count);
>  
> -	ieee80211_txq_teardown_flows(local);


This is after shutting down interfaces.

> @@ -1469,6 +1470,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
>  	 * because the driver cannot be handing us frames any
>  	 * more and the tasklet is killed.
>  	 */
> +	ieee80211_txq_teardown_flows(local);
>  	ieee80211_remove_interfaces(local);
> 

But now it's before. Why is that safe?

johannes

Re: [PATCH] wifi: mac80211: fix memory leak in ieee80211_register_hw()

[shaozhengchao]

On 2022/12/1 20:52, Johannes Berg wrote:
> On Tue, 2022-11-22 at 17:11 +0800, Zhengchao Shao wrote:
>>
>> +++ b/net/mac80211/iface.c
>> @@ -2326,8 +2326,6 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
>>   	WARN(local->open_count, "%s: open count remains %d\n",
>>   	     wiphy_name(local->hw.wiphy), local->open_count);
>>   
>> -	ieee80211_txq_teardown_flows(local);
> 
> 
> This is after shutting down interfaces.
> 
>> @@ -1469,6 +1470,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
>>   	 * because the driver cannot be handing us frames any
>>   	 * more and the tasklet is killed.
>>   	 */
>> +	ieee80211_txq_teardown_flows(local);
>>   	ieee80211_remove_interfaces(local);
>>
> 
> But now it's before. Why is that safe?
> 
> johannes
Hi johannes:
	Thank you for your review. This change may be unsafe.
Driver do clear its reource must be after hardware stop. Remove
it before shutdown interface is unsafe. I will change in V2.

Zhengchao Shao