From: Pavel Skripkin <paskripkin@gmail.com>
To: Dongliang Mu <mudongliangabcd@gmail.com>
Cc: steve.glendinning@shawell.net, davem@davemloft.net,
kuba@kernel.org, netdev@vger.kernel.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: usb: fix possible use-after-free in smsc75xx_bind
Date: Mon, 14 Jun 2021 19:00:45 +0300 [thread overview]
Message-ID: <20210614190045.5b4c92e6@gmail.com> (raw)
In-Reply-To: <20210614153712.2172662-1-mudongliangabcd@gmail.com>
On Mon, 14 Jun 2021 23:37:12 +0800
Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> The commit 46a8b29c6306 ("net: usb: fix memory leak in smsc75xx_bind")
> fails to clean up the work scheduled in smsc75xx_reset->
> smsc75xx_set_multicast, which leads to use-after-free if the work is
> scheduled to start after the deallocation. In addition, this patch
> also removes one dangling pointer - dev->data[0].
>
> This patch calls cancel_work_sync to cancel the schedule work and set
> the dangling pointer to NULL.
>
> Fixes: 46a8b29c6306 ("net: usb: fix memory leak in smsc75xx_bind")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
> drivers/net/usb/smsc75xx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
> index b286993da67c..f81740fcc8d5 100644
> --- a/drivers/net/usb/smsc75xx.c
> +++ b/drivers/net/usb/smsc75xx.c
> @@ -1504,7 +1504,10 @@ static int smsc75xx_bind(struct usbnet *dev,
> struct usb_interface *intf) return 0;
>
> err:
> + cancel_work_sync(&pdata->set_multicast);
> kfree(pdata);
> + pdata = NULL;
> + dev->data[0] = 0;
> return ret;
> }
>
Hi, Dongliang!
Just my thougth about this patch:
INIT_WORK(&pdata->set_multicast, smsc75xx_deferred_multicast_write);
does not queue anything, it just initalizes list structure and assigns
callback function. The actual work sheduling happens in
smsc75xx_set_multicast() which is smsc75xx_netdev_ops member.
In case of any error in smsc75xx_bind() the device registration fails
and smsc75xx_netdev_ops won't be registered, so, i guess, there is no
chance of UAF.
Am I missing something? :)
With regards,
Pavel Skripkin
next prev parent reply other threads:[~2021-06-14 16:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-14 15:37 [PATCH] net: usb: fix possible use-after-free in smsc75xx_bind Dongliang Mu
2021-06-14 16:00 ` Pavel Skripkin [this message]
2021-06-14 23:01 ` Dongliang Mu
2021-06-15 13:31 ` Pavel Skripkin
2021-06-16 2:16 ` Dongliang Mu
2021-06-15 7:38 ` Greg KH
2021-06-15 7:56 ` Dongliang Mu
2021-06-15 9:44 ` Greg KH
2021-06-15 10:10 ` Dongliang Mu
2021-06-15 10:24 ` Dongliang Mu
2021-06-15 11:12 ` Greg KH
2021-06-15 12:07 ` Dongliang Mu
2021-06-15 13:03 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210614190045.5b4c92e6@gmail.com \
--to=paskripkin@gmail.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mudongliangabcd@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=steve.glendinning@shawell.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.