All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
@ 2021-05-21 16:25 Dongliang Mu
  2021-05-21 16:29   ` 慕冬亮
  2021-05-21 17:02 ` Greg KH
  0 siblings, 2 replies; 6+ messages in thread
From: Dongliang Mu @ 2021-05-21 16:25 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, gregkh, rkovhaev,
	straube.linux, linux-staging, linux-kernel
  Cc: Dongliang Mu, syzbot+1c46f3771695bccbdb3a

r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
resource (e.g., struct urb) due to the failure of firmware loading.

Fix this by invoking r8712_free_drv_sw at the failure site.

Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
v1->v2: fix the initialization of pnetdev

 drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
index dc21e7743349..57e773464e18 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
 	struct usb_device *udev = interface_to_usbdev(pusb_intf);
 
 	if (pnetdev) {
+		struct net_device *newpnetdev = NULL;
 		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);
+		newpnetdev = usb_get_intfdata(pusb_intf);
 		usb_set_intfdata(pusb_intf, NULL);
-		if (!pnetdev)
+		if (!newpnetdev)
 			goto firmware_load_fail;
 		release_firmware(padapter->fw);
 		if (drvpriv.drv_registered)
@@ -625,6 +626,10 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
 	 */
 	if (udev->state != USB_STATE_NOTATTACHED)
 		usb_reset_device(udev);
+	if (pnetdev) {
+		struct _adapter *padapter = netdev_priv(pnetdev);
+		r8712_free_drv_sw(padapter);
+	}
 }
 
 static int __init r8712u_drv_entry(void)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
  2021-05-21 16:25 [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv Dongliang Mu
@ 2021-05-21 16:29   ` 慕冬亮
  2021-05-21 17:02 ` Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: 慕冬亮 @ 2021-05-21 16:29 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, Greg KH, rkovhaev,
	straube.linux, linux-staging, linux-kernel, Dmitry Vyukov
  Cc: syzbot+1c46f3771695bccbdb3a

On Sat, May 22, 2021 at 12:25 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
> resource (e.g., struct urb) due to the failure of firmware loading.
>
> Fix this by invoking r8712_free_drv_sw at the failure site.
>
> Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
> Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->v2: fix the initialization of pnetdev
>
>  drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> index dc21e7743349..57e773464e18 100644
> --- a/drivers/staging/rtl8712/usb_intf.c
> +++ b/drivers/staging/rtl8712/usb_intf.c
> @@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
>         struct usb_device *udev = interface_to_usbdev(pusb_intf);
>
>         if (pnetdev) {
> +               struct net_device *newpnetdev = NULL;
>                 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);
> +               newpnetdev = usb_get_intfdata(pusb_intf);
>                 usb_set_intfdata(pusb_intf, NULL);
> -               if (!pnetdev)
> +               if (!newpnetdev)
>                         goto firmware_load_fail;
>                 release_firmware(padapter->fw);
>                 if (drvpriv.drv_registered)
> @@ -625,6 +626,10 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
>          */
>         if (udev->state != USB_STATE_NOTATTACHED)
>                 usb_reset_device(udev);
> +       if (pnetdev) {
> +               struct _adapter *padapter = netdev_priv(pnetdev);
> +               r8712_free_drv_sw(padapter);
> +       }
>  }
>
>  static int __init r8712u_drv_entry(void)
> --
> 2.25.1
>

I tested my patch in my local workspace. No matter for the latest
upstream or f40ddce88593482919761f74910f42f4b84c004b (the kernel
version for the first crash in the syzbot), my local workspace both
show the memory leak disappears once the kernel is patched. However,
syzbot testing shows it still triggers the memory leak. @Dmitry Vyukov
Can you please help take a look? Maybe I incorrectly use this feature.
Thanks in advance.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
@ 2021-05-21 16:29   ` 慕冬亮
  0 siblings, 0 replies; 6+ messages in thread
From: 慕冬亮 @ 2021-05-21 16:29 UTC (permalink / raw)
  To: Larry.Finger, florian.c.schilhabel, Greg KH, rkovhaev,
	straube.linux, linux-staging, linux-kernel, Dmitry Vyukov
  Cc: syzbot+1c46f3771695bccbdb3a

On Sat, May 22, 2021 at 12:25 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
> resource (e.g., struct urb) due to the failure of firmware loading.
>
> Fix this by invoking r8712_free_drv_sw at the failure site.
>
> Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
> Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->v2: fix the initialization of pnetdev
>
>  drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> index dc21e7743349..57e773464e18 100644
> --- a/drivers/staging/rtl8712/usb_intf.c
> +++ b/drivers/staging/rtl8712/usb_intf.c
> @@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
>         struct usb_device *udev = interface_to_usbdev(pusb_intf);
>
>         if (pnetdev) {
> +               struct net_device *newpnetdev = NULL;
>                 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);
> +               newpnetdev = usb_get_intfdata(pusb_intf);
>                 usb_set_intfdata(pusb_intf, NULL);
> -               if (!pnetdev)
> +               if (!newpnetdev)
>                         goto firmware_load_fail;
>                 release_firmware(padapter->fw);
>                 if (drvpriv.drv_registered)
> @@ -625,6 +626,10 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
>          */
>         if (udev->state != USB_STATE_NOTATTACHED)
>                 usb_reset_device(udev);
> +       if (pnetdev) {
> +               struct _adapter *padapter = netdev_priv(pnetdev);
> +               r8712_free_drv_sw(padapter);
> +       }
>  }
>
>  static int __init r8712u_drv_entry(void)
> --
> 2.25.1
>

I tested my patch in my local workspace. No matter for the latest
upstream or f40ddce88593482919761f74910f42f4b84c004b (the kernel
version for the first crash in the syzbot), my local workspace both
show the memory leak disappears once the kernel is patched. However,
syzbot testing shows it still triggers the memory leak. @Dmitry Vyukov
Can you please help take a look? Maybe I incorrectly use this feature.
Thanks in advance.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
  2021-05-21 16:25 [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv Dongliang Mu
  2021-05-21 16:29   ` 慕冬亮
@ 2021-05-21 17:02 ` Greg KH
  2021-05-22  0:43     ` 慕冬亮
  1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2021-05-21 17:02 UTC (permalink / raw)
  To: Dongliang Mu
  Cc: Larry.Finger, florian.c.schilhabel, rkovhaev, straube.linux,
	linux-staging, linux-kernel, syzbot+1c46f3771695bccbdb3a

On Sat, May 22, 2021 at 12:25:19AM +0800, Dongliang Mu wrote:
> r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
> resource (e.g., struct urb) due to the failure of firmware loading.
> 
> Fix this by invoking r8712_free_drv_sw at the failure site.
> 
> Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
> Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> v1->v2: fix the initialization of pnetdev
> 
>  drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> index dc21e7743349..57e773464e18 100644
> --- a/drivers/staging/rtl8712/usb_intf.c
> +++ b/drivers/staging/rtl8712/usb_intf.c
> @@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
>  	struct usb_device *udev = interface_to_usbdev(pusb_intf);
>  
>  	if (pnetdev) {
> +		struct net_device *newpnetdev = NULL;
>  		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);
> +		newpnetdev = usb_get_intfdata(pusb_intf);

Please learn a bit more C knowledge before messing around in the kernel
tree.  Between the mistake in this chunk you added, and the mistake in
the previous submission, I think you need more experience here as there
are some things you still need to learn.

Best of luck!

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
  2021-05-21 17:02 ` Greg KH
@ 2021-05-22  0:43     ` 慕冬亮
  0 siblings, 0 replies; 6+ messages in thread
From: 慕冬亮 @ 2021-05-22  0:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Larry.Finger, florian.c.schilhabel, rkovhaev, straube.linux,
	linux-staging, linux-kernel, syzbot+1c46f3771695bccbdb3a

On Sat, May 22, 2021 at 1:02 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, May 22, 2021 at 12:25:19AM +0800, Dongliang Mu wrote:
> > r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
> > resource (e.g., struct urb) due to the failure of firmware loading.
> >
> > Fix this by invoking r8712_free_drv_sw at the failure site.
> >
> > Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
> > Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> > v1->v2: fix the initialization of pnetdev
> >
> >  drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> > index dc21e7743349..57e773464e18 100644
> > --- a/drivers/staging/rtl8712/usb_intf.c
> > +++ b/drivers/staging/rtl8712/usb_intf.c
> > @@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
> >       struct usb_device *udev = interface_to_usbdev(pusb_intf);
> >
> >       if (pnetdev) {
> > +             struct net_device *newpnetdev = NULL;
> >               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);
> > +             newpnetdev = usb_get_intfdata(pusb_intf);
>
> Please learn a bit more C knowledge before messing around in the kernel
> tree.  Between the mistake in this chunk you added, and the mistake in
> the previous submission, I think you need more experience here as there
> are some things you still need to learn.

I am a kernel newbie in developing patches for Linux kernel. Sorry for
my naive mistakes. I will learn more and then submit some "good"
patches.

BTW, I think I have pointed out the underlying bug here, you guys can
fix it yourself.

>
> Best of luck!
>
> greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv
@ 2021-05-22  0:43     ` 慕冬亮
  0 siblings, 0 replies; 6+ messages in thread
From: 慕冬亮 @ 2021-05-22  0:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Larry.Finger, florian.c.schilhabel, rkovhaev, straube.linux,
	linux-staging, linux-kernel, syzbot+1c46f3771695bccbdb3a

On Sat, May 22, 2021 at 1:02 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, May 22, 2021 at 12:25:19AM +0800, Dongliang Mu wrote:
> > r871xu_dev_remove failed to call r8712_free_drv_sw() and free the
> > resource (e.g., struct urb) due to the failure of firmware loading.
> >
> > Fix this by invoking r8712_free_drv_sw at the failure site.
> >
> > Reported-by: syzbot+1c46f3771695bccbdb3a@syzkaller.appspotmail.com
> > Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> > v1->v2: fix the initialization of pnetdev
> >
> >  drivers/staging/rtl8712/usb_intf.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
> > index dc21e7743349..57e773464e18 100644
> > --- a/drivers/staging/rtl8712/usb_intf.c
> > +++ b/drivers/staging/rtl8712/usb_intf.c
> > @@ -593,13 +593,14 @@ static void r871xu_dev_remove(struct usb_interface *pusb_intf)
> >       struct usb_device *udev = interface_to_usbdev(pusb_intf);
> >
> >       if (pnetdev) {
> > +             struct net_device *newpnetdev = NULL;
> >               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);
> > +             newpnetdev = usb_get_intfdata(pusb_intf);
>
> Please learn a bit more C knowledge before messing around in the kernel
> tree.  Between the mistake in this chunk you added, and the mistake in
> the previous submission, I think you need more experience here as there
> are some things you still need to learn.

I am a kernel newbie in developing patches for Linux kernel. Sorry for
my naive mistakes. I will learn more and then submit some "good"
patches.

BTW, I think I have pointed out the underlying bug here, you guys can
fix it yourself.

>
> Best of luck!
>
> greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-05-22  0:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 16:25 [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv Dongliang Mu
2021-05-21 16:29 ` 慕冬亮
2021-05-21 16:29   ` 慕冬亮
2021-05-21 17:02 ` Greg KH
2021-05-22  0:43   ` 慕冬亮
2021-05-22  0:43     ` 慕冬亮

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.