Re: Possible double-free in the usbnet driver
On Tue, Mar 8, 2016 at 12:39 AM, Oliver Neukum wrote: > On Mon, 2016-03-07 at 22:50 +0300, Andrey Konovalov wrote: >> Could you also add: >> Reported-by: Andrey Konovalov > > Well, the exact bug you reported is fixed in Bjorn's > patch the way Linus suggested. I'm fixing just a further > race that would require an error condition on top > of what you have seen. > So I don't think your Reported-by would be totally > appropriate. Oh, OK, Sorry. I thought this was a part of the same fix. > > Regards > Oliver > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Mon, 2016-03-07 at 22:50 +0300, Andrey Konovalov wrote: > Could you also add: > Reported-by: Andrey Konovalov Well, the exact bug you reported is fixed in Bjorn's patch the way Linus suggested. I'm fixing just a further race that would require an error condition on top of what you have seen. So I don't think your Reported-by would be totally appropriate. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
From: Andrey Konovalov Date: Mon, 7 Mar 2016 22:50:41 +0300 > On Mon, Mar 7, 2016 at 10:11 PM, David Miller wrote: >> From: Linus Torvalds >> Date: Mon, 7 Mar 2016 10:13:09 -0800 >> >>> On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork wrote: Definitely. The patch is so obviously correct that we can only wonder how it was possible to miss it it the first place :) Will take a look to see if we could do a better job cleaning up in other places. >>> >>> What should I do for 4.5? Will there be a pull request for this, or >>> should I just commit my cdc_ncm_bind() patch directly? >> >> Yes I plan to send you a pull request today with Oliver's fix. >> >> Assuming this is what you guys are referring to: >> >> commit 1666984c8625b3db19a9abc298931d35ab7bc64b >> Author: Oliver Neukum >> Date: Mon Mar 7 11:31:10 2016 +0100 >> >> usbnet: cleanup after bind() in probe() >> >> In case bind() works, but a later error forces bailing >> in probe() in error cases work and a timer may be scheduled. >> They must be killed. This fixes an error case related to >> the double free reported in >> http://www.spinics.net/lists/netdev/msg367669.html >> and needs to go on top of Linus' fix to cdc-ncm. >> >> Signed-off-by: Oliver Neukum >> Signed-off-by: David S. Miller > > Could you also add: > Reported-by: Andrey Konovalov > ? Sorry it's already committed to my tree and I can't redo the commit message once that happens since my tree has static history. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Mon, Mar 7, 2016 at 10:11 PM, David Miller wrote: > From: Linus Torvalds > Date: Mon, 7 Mar 2016 10:13:09 -0800 > >> On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork wrote: >>> >>> >>> Definitely. The patch is so obviously correct that we can only wonder how >>> it was possible to miss it it the first place :) >>> >>> Will take a look to see if we could do a better job cleaning up in other >>> places. >> >> What should I do for 4.5? Will there be a pull request for this, or >> should I just commit my cdc_ncm_bind() patch directly? > > Yes I plan to send you a pull request today with Oliver's fix. > > Assuming this is what you guys are referring to: > > commit 1666984c8625b3db19a9abc298931d35ab7bc64b > Author: Oliver Neukum > Date: Mon Mar 7 11:31:10 2016 +0100 > > usbnet: cleanup after bind() in probe() > > In case bind() works, but a later error forces bailing > in probe() in error cases work and a timer may be scheduled. > They must be killed. This fixes an error case related to > the double free reported in > http://www.spinics.net/lists/netdev/msg367669.html > and needs to go on top of Linus' fix to cdc-ncm. > > Signed-off-by: Oliver Neukum > Signed-off-by: David S. Miller Could you also add: Reported-by: Andrey Konovalov ? Thanks in advance. > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > index 0b0ba7e..1079812 100644 > --- a/drivers/net/usb/usbnet.c > +++ b/drivers/net/usb/usbnet.c > @@ -1769,6 +1769,13 @@ out3: > if (info->unbind) > info->unbind (dev, udev); > out1: > + /* subdrivers must undo all they did in bind() if they > +* fail it, but we may fail later and a deferred kevent > +* may trigger an error resubmitting itself and, worse, > +* schedule a timer. So we kill it all just in case. > +*/ > + cancel_work_sync(&dev->kevent); > + del_timer_sync(&dev->delay); > free_netdev(net); > out: > return status; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
From: Linus Torvalds Date: Mon, 7 Mar 2016 10:13:09 -0800 > On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork wrote: >> >> >> Definitely. The patch is so obviously correct that we can only wonder how >> it was possible to miss it it the first place :) >> >> Will take a look to see if we could do a better job cleaning up in other >> places. > > What should I do for 4.5? Will there be a pull request for this, or > should I just commit my cdc_ncm_bind() patch directly? Yes I plan to send you a pull request today with Oliver's fix. Assuming this is what you guys are referring to: commit 1666984c8625b3db19a9abc298931d35ab7bc64b Author: Oliver Neukum Date: Mon Mar 7 11:31:10 2016 +0100 usbnet: cleanup after bind() in probe() In case bind() works, but a later error forces bailing in probe() in error cases work and a timer may be scheduled. They must be killed. This fixes an error case related to the double free reported in http://www.spinics.net/lists/netdev/msg367669.html and needs to go on top of Linus' fix to cdc-ncm. Signed-off-by: Oliver Neukum Signed-off-by: David S. Miller diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 0b0ba7e..1079812 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1769,6 +1769,13 @@ out3: if (info->unbind) info->unbind (dev, udev); out1: + /* subdrivers must undo all they did in bind() if they +* fail it, but we may fail later and a deferred kevent +* may trigger an error resubmitting itself and, worse, +* schedule a timer. So we kill it all just in case. +*/ + cancel_work_sync(&dev->kevent); + del_timer_sync(&dev->delay); free_netdev(net); out: return status; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Sat, Mar 5, 2016 at 11:53 AM, Bjørn Mork wrote: > > > Definitely. The patch is so obviously correct that we can only wonder how it > was possible to miss it it the first place :) > > Will take a look to see if we could do a better job cleaning up in other > places. What should I do for 4.5? Will there be a pull request for this, or should I just commit my cdc_ncm_bind() patch directly? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Fri, Mar 4, 2016 at 11:43 PM, Linus Torvalds wrote: > On Fri, Mar 4, 2016 at 2:26 PM, Andrey Konovalov wrote: >> >> and when I run the vm and connect the device I get: >> >> [ 23.672662] cdc_ncm 1-1:1.6: bind() failure >> [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 >> [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 >> >> So this seems to be a double-free (or at least a double free_netdev() >> call), but the object gets freed twice from usbnet_probe() and not >> from usbnet_disconnect(), so you're right that the latter doesn't get >> called. I'm not sure how usbnet_probe() ends up being called twice. > > I still don't think it's a double free. I think the probe thing is > called twice, and that's why to different allocations get free'd twice > (and the reason it's the same pointer is that the same allocation got > reused) FYI, we have a KASAN patch in flight that adds "quarantine" for freed memory (memory is reused only after a significant delay). It should help to easily differentiate between use-after-free, double-free and heap-out-of-bound. Yes, now it is confusing. > But I don't know that driver, really. > >>> Which particular usbnet bind routine is this? There are multiple >>> sub-drivers for usbnet that all do different things. >> >> cdc_ncm_bind() > > Ok, so that matches my theory. > > Does this attached stupid patch make the warning go away? Because from > what I can tell, usbnet_link_change() really will start using that > "dev" that simply isn't valid - and will be free'd - if the bind > fails. > > So you have usbnet_defer_kevent() getting triggered, which in turn > ends up using "usbnet->kevent" > > But somebody like Oliver is really the right person to check this. For > example, it's entirely possible that we should just instead do > > cancel_work_sync(&dev->kevent); > > before the "free_netdev(net)" in the "out1" label. > > And there might be other things that bind() can have touched than the > kevent workqueue. > >Linus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On March 5, 2016 4:51:30 PM CET, Oliver Neukum wrote: >On Fri, 2016-03-04 at 14:43 -0800, Linus Torvalds wrote: > >> So you have usbnet_defer_kevent() getting triggered, which in turn >> ends up using "usbnet->kevent" >> >> But somebody like Oliver is really the right person to check this. >For >> example, it's entirely possible that we should just instead do >> >> cancel_work_sync(&dev->kevent); >> >> before the "free_netdev(net)" in the "out1" label. > >Hi Bjorn, > >I thinbk Linus has analyzed this correctly, but the fix really needs >to cancel the work, because we can also fail later after bind() has >already run. However, still cdc-ncm and the other drivers should clean >up after themselves if bind() fails, as usbnet really cannot known what >the subdrivers have done. > >So in conclusion, I think Linus' fix should also go into cdc-ncm. Definitely. The patch is so obviously correct that we can only wonder how it was possible to miss it it the first place :) Will take a look to see if we could do a better job cleaning up in other places. (I do also wonder a bit about the failure to bind - is that expected or is there some bug in the cdc_ncm descriptor parsing?) Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Fri, 2016-03-04 at 14:43 -0800, Linus Torvalds wrote: > So you have usbnet_defer_kevent() getting triggered, which in turn > ends up using "usbnet->kevent" > > But somebody like Oliver is really the right person to check this. For > example, it's entirely possible that we should just instead do > > cancel_work_sync(&dev->kevent); > > before the "free_netdev(net)" in the "out1" label. Hi Bjorn, I thinbk Linus has analyzed this correctly, but the fix really needs to cancel the work, because we can also fail later after bind() has already run. However, still cdc-ncm and the other drivers should clean up after themselves if bind() fails, as usbnet really cannot known what the subdrivers have done. So in conclusion, I think Linus' fix should also go into cdc-ncm. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Sat, Mar 5, 2016 at 2:00 AM, Andrey Konovalov wrote: > On Sat, Mar 5, 2016 at 1:42 AM, Oliver Neukum wrote: >> On Sat, 2016-03-05 at 01:26 +0300, Andrey Konovalov wrote: >>> and when I run the vm and connect the device I get: >>> >>> [ 23.672662] cdc_ncm 1-1:1.6: bind() failure >>> [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 >>> [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 >>> >>> So this seems to be a double-free (or at least a double free_netdev() >>> call), but the object gets freed twice from usbnet_probe() and not >>> from usbnet_disconnect(), so you're right that the latter doesn't get >>> called. I'm not sure how usbnet_probe() ends up being called twice. >> >> Do you have lsusb? > > You mean inside the vm? > I do. Or did you want the faulty device descriptor itself? I used this: Speed High Bus 004 Device 003: ID 0bdb:1911 Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB2.00 bDeviceClass 2 Communications bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor0x idProduct 0x bcdDevice 0.00 iManufacturer1 iProduct 2 iSerial 3 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 371 bNumInterfaces 11 bConfigurationValue 1 iConfiguration 4 bmAttributes 0xe0 Self Powered Remote Wakeup bMaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 6 bAlternateSetting0 bNumEndpoints1 bInterfaceClass 2 Communications bInterfaceSubClass 13 bInterfaceProtocol 0 iInterface 11 > >> >> Regards >> Oliver >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Sat, Mar 5, 2016 at 1:42 AM, Oliver Neukum wrote: > On Sat, 2016-03-05 at 01:26 +0300, Andrey Konovalov wrote: >> and when I run the vm and connect the device I get: >> >> [ 23.672662] cdc_ncm 1-1:1.6: bind() failure >> [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 >> [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 >> >> So this seems to be a double-free (or at least a double free_netdev() >> call), but the object gets freed twice from usbnet_probe() and not >> from usbnet_disconnect(), so you're right that the latter doesn't get >> called. I'm not sure how usbnet_probe() ends up being called twice. > > Do you have lsusb? You mean inside the vm? I do. > > Regards > Oliver > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Sat, Mar 5, 2016 at 1:43 AM, Linus Torvalds wrote: > On Fri, Mar 4, 2016 at 2:26 PM, Andrey Konovalov wrote: >> >> and when I run the vm and connect the device I get: >> >> [ 23.672662] cdc_ncm 1-1:1.6: bind() failure >> [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 >> [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 >> >> So this seems to be a double-free (or at least a double free_netdev() >> call), but the object gets freed twice from usbnet_probe() and not >> from usbnet_disconnect(), so you're right that the latter doesn't get >> called. I'm not sure how usbnet_probe() ends up being called twice. > > I still don't think it's a double free. I think the probe thing is > called twice, and that's why to different allocations get free'd twice > (and the reason it's the same pointer is that the same allocation got > reused) > > But I don't know that driver, really. > >>> Which particular usbnet bind routine is this? There are multiple >>> sub-drivers for usbnet that all do different things. >> >> cdc_ncm_bind() > > Ok, so that matches my theory. > > Does this attached stupid patch make the warning go away? Because from > what I can tell, usbnet_link_change() really will start using that > "dev" that simply isn't valid - and will be free'd - if the bind > fails. Yes, KASAN doesn't report anything with your patch. > > So you have usbnet_defer_kevent() getting triggered, which in turn > ends up using "usbnet->kevent" > > But somebody like Oliver is really the right person to check this. For > example, it's entirely possible that we should just instead do > > cancel_work_sync(&dev->kevent); > > before the "free_netdev(net)" in the "out1" label. > > And there might be other things that bind() can have touched than the > kevent workqueue. > >Linus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Sat, 2016-03-05 at 01:26 +0300, Andrey Konovalov wrote: > and when I run the vm and connect the device I get: > > [ 23.672662] cdc_ncm 1-1:1.6: bind() failure > [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 > [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 > > So this seems to be a double-free (or at least a double free_netdev() > call), but the object gets freed twice from usbnet_probe() and not > from usbnet_disconnect(), so you're right that the latter doesn't get > called. I'm not sure how usbnet_probe() ends up being called twice. Do you have lsusb? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
On Fri, Mar 4, 2016 at 2:26 PM, Andrey Konovalov wrote: > > and when I run the vm and connect the device I get: > > [ 23.672662] cdc_ncm 1-1:1.6: bind() failure > [ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000 > [ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000 > > So this seems to be a double-free (or at least a double free_netdev() > call), but the object gets freed twice from usbnet_probe() and not > from usbnet_disconnect(), so you're right that the latter doesn't get > called. I'm not sure how usbnet_probe() ends up being called twice. I still don't think it's a double free. I think the probe thing is called twice, and that's why to different allocations get free'd twice (and the reason it's the same pointer is that the same allocation got reused) But I don't know that driver, really. >> Which particular usbnet bind routine is this? There are multiple >> sub-drivers for usbnet that all do different things. > > cdc_ncm_bind() Ok, so that matches my theory. Does this attached stupid patch make the warning go away? Because from what I can tell, usbnet_link_change() really will start using that "dev" that simply isn't valid - and will be free'd - if the bind fails. So you have usbnet_defer_kevent() getting triggered, which in turn ends up using "usbnet->kevent" But somebody like Oliver is really the right person to check this. For example, it's entirely possible that we should just instead do cancel_work_sync(&dev->kevent); before the "free_netdev(net)" in the "out1" label. And there might be other things that bind() can have touched than the kevent workqueue. Linus drivers/net/usb/cdc_ncm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index dc0212c3cc28..5878b54cc563 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -995,6 +995,8 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf) * placed NDP. */ ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0); + if (ret < 0) + return ret; /* * We should get an event when network connection is "connected" or @@ -1003,7 +1005,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf) * start IPv6 negotiation and more. */ usbnet_link_change(dev, 0, 0); - return ret; + return 0; } static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder, size_t max)
Re: Possible double-free in the usbnet driver
On Sat, Mar 5, 2016 at 12:26 AM, Linus Torvalds
wrote:
> [ Moving this to proper lists ]
>
> On Thu, Mar 3, 2016 at 4:19 PM, Andrey Konovalov wrote:
>>
>> I found another double-free, this time in the usbnet driver.
>
> Hmm. It doesn't look like a double free to me, at least from the logs
> you attached.
>
>> Whenever the `bind()` function fails (drivers/net/usb/usbnet.c:1676) when
>> called from `usbnet_probe()` (and it can fail due to a invalid usb
>> descriptor),
>> `free_netdev()` is called for the `net` device
>> (drivers/net/usb/usbnet.c:1772).
>> Then, `free_netdev(net)` is called again in `usbnet_disconnect()`
>> (drivers/net/usb/usbnet.c:1570) causing a double-free.
>
> The KASAN report says that it's a use-after-free in the kworker
> thread: the net device got free'd at the end of usbnet_probe(), but
> some work-struct was apparently active at the time.
>
> There might be a double free later that isn't in your report, though.
> Do you have the data for that?
I uploaded full kernel log here:
https://gist.github.com/xairy/6a244871959187209fdb
My initial idea was that an object is freed by free_netdev(), then
allocated somewhere during execution of kworker-related code and then
this object gets freed by free_netdev() again and that's why we have
such use-after-free reports. That didn't explain the deallocation
stack trace in the report, but I thought this was due to a
racy-use-after-free.
I just added the following debug output:
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 0b0ba7e..f7e1415 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1567,6 +1567,7 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_free_urb(dev->interrupt);
kfree(dev->padding_pkt);
+ pr_err("usbnet_disconnect(): freeing netdev: %p\n", net);
free_netdev(net);
}
EXPORT_SYMBOL_GPL(usbnet_disconnect);
@@ -1769,6 +1770,7 @@ out3:
if (info->unbind)
info->unbind (dev, udev);
out1:
+ pr_err("usbnet_probe(): freeing netdev: %p\n", net);
free_netdev(net);
out:
return status;
and when I run the vm and connect the device I get:
[ 23.672662] cdc_ncm 1-1:1.6: bind() failure
[ 23.673447] usbnet_probe(): freeing netdev: 88006ab48000
[ 23.675822] usbnet_probe(): freeing netdev: 88006ab48000
So this seems to be a double-free (or at least a double free_netdev()
call), but the object gets freed twice from usbnet_probe() and not
from usbnet_disconnect(), so you're right that the latter doesn't get
called. I'm not sure how usbnet_probe() ends up being called twice.
>
> But I didn't think we even called the disconnect() function if the
> "bind()" failed, so I don't think that one should free it. Greg?
>
> So it *sounds* to me like the usbnet "bind()" routine ended up
> returning an error, but doing so *after* it had already activated the
> structure somehow.
>
> Which particular usbnet bind routine is this? There are multiple
> sub-drivers for usbnet that all do different things.
cdc_ncm_bind()
>
> For example, it *looks* like the cdc_ncm_bind() will have done a
> usbnet_link_change() even if the bind fails. So now we've done a
> usbnet_defer_kevent() even though we're failing, and then that sets
> the ball rolling to later touch the netdev that we're freeing due to
> the failure.
>
> But I may be *entirely* misreading this thing.
>
> Anyway, I'm cc'ing the usbnet people who actually know the code (and netdev).
>
> The proper fix may be to just cancel any work that might have been set
> up before freeing. Or maybe that netdev *does* get free'd later some
> other way properly. Let's see what the experts on the usbnet driver
> say.
>
> Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible double-free in the usbnet driver
[ Moving this to proper lists ] On Thu, Mar 3, 2016 at 4:19 PM, Andrey Konovalov wrote: > > I found another double-free, this time in the usbnet driver. Hmm. It doesn't look like a double free to me, at least from the logs you attached. > Whenever the `bind()` function fails (drivers/net/usb/usbnet.c:1676) when > called from `usbnet_probe()` (and it can fail due to a invalid usb > descriptor), > `free_netdev()` is called for the `net` device > (drivers/net/usb/usbnet.c:1772). > Then, `free_netdev(net)` is called again in `usbnet_disconnect()` > (drivers/net/usb/usbnet.c:1570) causing a double-free. The KASAN report says that it's a use-after-free in the kworker thread: the net device got free'd at the end of usbnet_probe(), but some work-struct was apparently active at the time. There might be a double free later that isn't in your report, though. Do you have the data for that? But I didn't think we even called the disconnect() function if the "bind()" failed, so I don't think that one should free it. Greg? So it *sounds* to me like the usbnet "bind()" routine ended up returning an error, but doing so *after* it had already activated the structure somehow. Which particular usbnet bind routine is this? There are multiple sub-drivers for usbnet that all do different things. For example, it *looks* like the cdc_ncm_bind() will have done a usbnet_link_change() even if the bind fails. So now we've done a usbnet_defer_kevent() even though we're failing, and then that sets the ball rolling to later touch the netdev that we're freeing due to the failure. But I may be *entirely* misreading this thing. Anyway, I'm cc'ing the usbnet people who actually know the code (and netdev). The proper fix may be to just cancel any work that might have been set up before freeing. Or maybe that netdev *does* get free'd later some other way properly. Let's see what the experts on the usbnet driver say. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
