* [PATCH 0/3] staging: rtl8712: code improvment and bug fixes
@ 2021-06-13 21:59 Pavel Skripkin
2021-06-13 22:00 ` [PATCH 1/3] staging: rtl8712: remove redundant check in r871xu_drv_init Pavel Skripkin
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-13 21:59 UTC (permalink / raw)
To: Larry.Finger, florian.c.schilhabel, gregkh
Cc: linux-staging, linux-kernel, Pavel Skripkin
In this patch series I fixed 2 memory leaks in rtl8712 driver and
made some code improvment. In the fisrt patch I removed redundant
branching to improve code speed and make it more straightforward.
The folowing 2 pathes fix 2 memory leaks in this driver. The first one
fixes memory leak in the error handling path in r871xu_drv_init and the
second one fixes memory leak in case of fw load failure.
Pavel Skripkin (3):
staging: rtl8712: remove redundant check in r871xu_drv_init
staging: rtl8712: fix error handling in r871xu_drv_init
staging: rtl8712: fix memory leak in rtl871x_load_fw_cb
drivers/staging/rtl8712/hal_init.c | 3 +++
drivers/staging/rtl8712/os_intfs.c | 4 ----
drivers/staging/rtl8712/usb_intf.c | 32 ++++++++++++++++--------------
3 files changed, 20 insertions(+), 19 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] staging: rtl8712: remove redundant check in r871xu_drv_init
2021-06-13 21:59 [PATCH 0/3] staging: rtl8712: code improvment and bug fixes Pavel Skripkin
@ 2021-06-13 22:00 ` Pavel Skripkin
2021-06-13 22:00 ` [PATCH 2/3] staging: rtl8712: fix error handling " Pavel Skripkin
2021-06-13 22:00 ` [PATCH 3/3] staging: rtl8712: fix memory leak in rtl871x_load_fw_cb Pavel Skripkin
2 siblings, 0 replies; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-13 22:00 UTC (permalink / raw)
To: Larry.Finger, florian.c.schilhabel, gregkh
Cc: linux-staging, linux-kernel, Pavel Skripkin
padapter->dvobj_init is initialized rigth before
initialization check. There is no need for any
branching here.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/rtl8712/usb_intf.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
index dc21e7743349..c482a6d1998b 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -380,13 +380,11 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
/* step 3.
* initialize the dvobj_priv
*/
- if (!padapter->dvobj_init) {
+
+ status = padapter->dvobj_init(padapter);
+ if (status != _SUCCESS)
goto error;
- } else {
- status = padapter->dvobj_init(padapter);
- if (status != _SUCCESS)
- goto error;
- }
+
/* step 4. */
status = r8712_init_drv_sw(padapter);
if (status)
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] staging: rtl8712: fix error handling in r871xu_drv_init
2021-06-13 21:59 [PATCH 0/3] staging: rtl8712: code improvment and bug fixes Pavel Skripkin
2021-06-13 22:00 ` [PATCH 1/3] staging: rtl8712: remove redundant check in r871xu_drv_init Pavel Skripkin
@ 2021-06-13 22:00 ` Pavel Skripkin
2021-06-13 22:00 ` [PATCH 3/3] staging: rtl8712: fix memory leak in rtl871x_load_fw_cb Pavel Skripkin
2 siblings, 0 replies; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-13 22:00 UTC (permalink / raw)
To: Larry.Finger, florian.c.schilhabel, gregkh
Cc: linux-staging, linux-kernel, Pavel Skripkin
Previous error handling path was unique for all
possible errors and there was unnecessary branching.
Also, one step for freeing drv_sw was missing. All
these problems was fixed by restructuring error
handling path.
Also, moved out free_netdev() from r8712_free_drv_sw() for
correct error handling.
Fixes: 2865d42c78a9 ("staging: r8712u: Add the new driver to the mainline kernel")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/rtl8712/os_intfs.c | 4 ----
drivers/staging/rtl8712/usb_intf.c | 22 +++++++++++++---------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index 0c3ae8495afb..2214aca09730 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -328,8 +328,6 @@ int r8712_init_drv_sw(struct _adapter *padapter)
void r8712_free_drv_sw(struct _adapter *padapter)
{
- struct net_device *pnetdev = padapter->pnetdev;
-
r8712_free_cmd_priv(&padapter->cmdpriv);
r8712_free_evt_priv(&padapter->evtpriv);
r8712_DeInitSwLeds(padapter);
@@ -339,8 +337,6 @@ void r8712_free_drv_sw(struct _adapter *padapter)
_r8712_free_sta_priv(&padapter->stapriv);
_r8712_free_recv_priv(&padapter->recvpriv);
mp871xdeinit(padapter);
- if (pnetdev)
- free_netdev(pnetdev);
}
static void enable_video_mode(struct _adapter *padapter, int cbw40_value)
diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
index c482a6d1998b..f97830049fc8 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -361,7 +361,7 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
/* step 1. */
pnetdev = r8712_init_netdev();
if (!pnetdev)
- goto error;
+ goto put_dev;
padapter = netdev_priv(pnetdev);
disable_ht_for_spec_devid(pdid, padapter);
pdvobjpriv = &padapter->dvobjpriv;
@@ -383,12 +383,12 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
status = padapter->dvobj_init(padapter);
if (status != _SUCCESS)
- goto error;
+ goto free_netdev;
/* step 4. */
status = r8712_init_drv_sw(padapter);
if (status)
- goto error;
+ goto dvobj_deinit;
/* step 5. read efuse/eeprom data and get mac_addr */
{
int i, offset;
@@ -568,17 +568,20 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
}
/* step 6. Load the firmware asynchronously */
if (rtl871x_load_fw(padapter))
- goto error;
+ goto deinit_drv_sw;
spin_lock_init(&padapter->lock_rx_ff0_filter);
mutex_init(&padapter->mutex_start);
return 0;
-error:
+
+deinit_drv_sw:
+ r8712_free_drv_sw(padapter);
+dvobj_deinit:
+ padapter->dvobj_deinit(padapter);
+free_netdev:
+ free_netdev(pnetdev);
+put_dev:
usb_put_dev(udev);
usb_set_intfdata(pusb_intf, NULL);
- if (padapter && padapter->dvobj_deinit)
- padapter->dvobj_deinit(padapter);
- if (pnetdev)
- free_netdev(pnetdev);
return -ENODEV;
}
@@ -610,6 +613,7 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
r8712_stop_drv_timers(padapter);
r871x_dev_unload(padapter);
r8712_free_drv_sw(padapter);
+ free_netdev(pnetdev);
/* decrease the reference count of the usb device structure
* when disconnect
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] staging: rtl8712: fix memory leak in rtl871x_load_fw_cb
2021-06-13 21:59 [PATCH 0/3] staging: rtl8712: code improvment and bug fixes Pavel Skripkin
2021-06-13 22:00 ` [PATCH 1/3] staging: rtl8712: remove redundant check in r871xu_drv_init Pavel Skripkin
2021-06-13 22:00 ` [PATCH 2/3] staging: rtl8712: fix error handling " Pavel Skripkin
@ 2021-06-13 22:00 ` Pavel Skripkin
2 siblings, 0 replies; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-13 22:00 UTC (permalink / raw)
To: Larry.Finger, florian.c.schilhabel, gregkh
Cc: linux-staging, linux-kernel, Pavel Skripkin
There is a leak in rtl8712 driver.
The problem was in non-freed adapter data if
firmware load failed.
This leak can be reproduced with this code:
https://syzkaller.appspot.com/text?tag=ReproC&x=16612f02d00000,
Autoload must fail (to not hit memory leak reported by syzkaller)
There are 2 possible ways how rtl871x_load_fw_cb() and
r871xu_dev_remove() can be called (in case of fw load error).
1st case:
r871xu_dev_remove() then rtl871x_load_fw_cb()
In this case r871xu_dev_remove() will wait for
completion and then will jump to the end, because
rtl871x_load_fw_cb() set intfdata to NULL:
if (pnetdev) {
struct _adapter *padapter = netdev_priv(pnetdev);
/* never exit with a firmware callback pending */
wait_for_completion(&padapter->rtl8712_fw_ready);
pnetdev = usb_get_intfdata(pusb_intf);
usb_set_intfdata(pusb_intf, NULL);
if (!pnetdev)
goto firmware_load_fail;
... clean up code here ...
}
2nd case:
rtl871x_load_fw_cb() then r871xu_dev_remove()
In this case pnetdev (from code snippet above) will
be zero (because rtl871x_load_fw_cb() set it to NULL)
And clean up code won't be executed again.
So, in all cases we need to free adapted data in rtl871x_load_fw_cb(),
because disconnect function cannot take care of it. And there won't be
any race conditions, because complete() call happens after setting
intfdata to NULL.
In previous patch I moved out free_netdev() from r8712_free_drv_sw()
and that's why now it's possible to free adapter data and then call
complete.
Fixes: 8c213fa59199 ("staging: r8712u: Use asynchronous firmware loading")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/rtl8712/hal_init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c
index 715f1fe8b472..22974277afa0 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -40,7 +40,10 @@ static void rtl871x_load_fw_cb(const struct firmware *firmware, void *context)
dev_err(&udev->dev, "r8712u: Firmware request failed\n");
usb_put_dev(udev);
usb_set_intfdata(usb_intf, NULL);
+ r8712_free_drv_sw(adapter);
+ adapter->dvobj_deinit(adapter);
complete(&adapter->rtl8712_fw_ready);
+ free_netdev(adapter->pnetdev);
return;
}
adapter->fw = firmware;
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-13 22:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-13 21:59 [PATCH 0/3] staging: rtl8712: code improvment and bug fixes Pavel Skripkin
2021-06-13 22:00 ` [PATCH 1/3] staging: rtl8712: remove redundant check in r871xu_drv_init Pavel Skripkin
2021-06-13 22:00 ` [PATCH 2/3] staging: rtl8712: fix error handling " Pavel Skripkin
2021-06-13 22:00 ` [PATCH 3/3] staging: rtl8712: fix memory leak in rtl871x_load_fw_cb Pavel Skripkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.