2022-11-22 09:06:12

by shaozhengchao

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

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 <[email protected]>
---
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


2022-12-01 12:59:12

by Johannes Berg

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

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

2022-12-02 02:54:23

by shaozhengchao

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



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