On 4/17/23 01:08, Ping-Ke Shih wrote:
-----Original Message-----
From: Larry Finger <larry.finger@xxxxxxxxx> On Behalf Of Larry Finger
Sent: Saturday, April 15, 2023 9:14 AM
To: Kalle Valo <kvalo@xxxxxxxxxx>
Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx; Larry Finger
<Larry.Finger@xxxxxxxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
Subject: [PATCH] wifi: rtw88: Fix memory leak in rtw88_usb
Kmemleak shows the following leak arising from routine in the usb
probe routine:
unreferenced object 0xffff895cb29bba00 (size 512):
comm "(udev-worker)", pid 534, jiffies 4294903932 (age 102751.088s)
hex dump (first 32 bytes):
77 30 30 30 00 00 00 00 02 2f 2d 2b 30 00 00 00 w000...../-+0...
02 00 2a 28 00 00 00 00 ff 55 ff ff ff 00 00 00 ..*(.....U......
backtrace:
[<ffffffff9265fa36>] kmalloc_trace+0x26/0x90
[<ffffffffc17eec41>] rtw_usb_probe+0x2f1/0x680 [rtw_usb]
[<ffffffffc03e19fd>] usb_probe_interface+0xdd/0x2e0 [usbcore]
[<ffffffff92b4f2fe>] really_probe+0x18e/0x3d0
[<ffffffff92b4f5b8>] __driver_probe_device+0x78/0x160
[<ffffffff92b4f6bf>] driver_probe_device+0x1f/0x90
[<ffffffff92b4f8df>] __driver_attach+0xbf/0x1b0
[<ffffffff92b4d350>] bus_for_each_dev+0x70/0xc0
[<ffffffff92b4e51e>] bus_add_driver+0x10e/0x210
[<ffffffff92b50935>] driver_register+0x55/0xf0
[<ffffffffc03e0708>] usb_register_driver+0x88/0x140 [usbcore]
[<ffffffff92401153>] do_one_initcall+0x43/0x210
[<ffffffff9254f42a>] do_init_module+0x4a/0x200
[<ffffffff92551d1c>] __do_sys_finit_module+0xac/0x120
[<ffffffff92ee6626>] do_syscall_64+0x56/0x80
[<ffffffff9300006a>] entry_SYSCALL_64_after_hwframe+0x46/0xb0
The leak was verified to be real by unloading the driver, which resulted
in a dangling pointer to the allocation.
The allocated memory is freed in rtw_usb_disconnect().
Signed-off-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
Cc: Ping-Ke Shih <pkshih@xxxxxxxxxxx>
---
drivers/net/wireless/realtek/rtw88/usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 68e1b782d199..d28493a8f16c 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -882,6 +882,7 @@ void rtw_usb_disconnect(struct usb_interface *intf)
rtw_unregister_hw(rtwdev, hw);
rtw_usb_deinit_tx(rtwdev);
rtw_usb_deinit_rx(rtwdev);
+ kfree(rtwusb->usb_data);
I suggest to do this in rtw_usb_intf_deinit() instead, because rtwusb->usb_data is
allocated by rtw_usb_intf_init(). Not only make the code symmetric also can handle
error cases properly of rtw_usb_probe().
Ping-Ke,
Thanks for the suggestion. I chose my location because the allocation was just
before the call to rtw_usb_init_tx(), thus the free went after
rtw_usb_deinit_tx(), but I see your point. I will submit v2.
Larry
