All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Dongliang Mu <mudongliangabcd@gmail.com>
Cc: Steve Glendinning <steve.glendinning@shawell.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Pavel Skripkin <paskripkin@gmail.com>,
	netdev@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net: usb: fix possible use-after-free in smsc75xx_bind
Date: Tue, 15 Jun 2021 13:12:20 +0200	[thread overview]
Message-ID: <YMiLFFRfXfBHpfAF@kroah.com> (raw)
In-Reply-To: <CAD-N9QW6LhRO+D-rr4xCCuq+m=jtD7LS_+GDVs9DkHe5paeSOg@mail.gmail.com>

On Tue, Jun 15, 2021 at 06:24:17PM +0800, Dongliang Mu wrote:
> On Tue, Jun 15, 2021 at 6:10 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
> >
> > On Tue, Jun 15, 2021 at 5:44 PM Greg KH <greg@kroah.com> wrote:
> > >
> > > On Tue, Jun 15, 2021 at 03:56:32PM +0800, Dongliang Mu wrote:
> > > > On Tue, Jun 15, 2021 at 3:38 PM Greg KH <greg@kroah.com> wrote:
> > > > >
> > > > > On Mon, Jun 14, 2021 at 11:37:12PM +0800, Dongliang Mu 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;
> > > > >
> > > > > Why do you have to set pdata to NULL afterward?
> > > > >
> > > >
> > > > It does not have to. pdata will be useless when the function exits. I
> > > > just referred to the implementation of smsc75xx_unbind.
> > >
> > > It's wrong there too :)
> >
> > /: I will fix such two sites in the v2 patch.
> 
> Hi gregkh,
> 
> If the schedule_work is not invoked, can I call
> ``cancel_work_sync(&pdata->set_multicast)''?

Why can you not call this then?

Did you try it and see?

thanks,

greg k-h

  reply	other threads:[~2021-06-15 11:12 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
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 [this message]
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=YMiLFFRfXfBHpfAF@kroah.com \
    --to=greg@kroah.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=paskripkin@gmail.com \
    --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.