linux-kernel - Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 May 2021 00:29:39 +0800
From:   慕冬亮 <mudongliangabcd@...il.com>
To:     Larry.Finger@...inger.net, florian.c.schilhabel@...glemail.com,
        Greg KH <gregkh@...uxfoundation.org>, rkovhaev@...il.com,
        straube.linux@...il.com, linux-staging@...ts.linux.dev,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>
Cc:     syzbot+1c46f3771695bccbdb3a@...kaller.appspotmail.com
Subject: Re: [PATCH v2] staging: rtl8712: Fix memory leak in r8712_init_recv_priv

On Sat, May 22, 2021 at 12:25 AM Dongliang Mu <mudongliangabcd@...il.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@...kaller.appspotmail.com
> Fixes: b4383c971bc5 ("staging: rtl8712: handle firmware load failure")
> Signed-off-by: Dongliang Mu <mudongliangabcd@...il.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.