netdev - Re: Possible double-free in the usbnet driver

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]

Message-ID: <DBDB517D-E4A4-4422-AECE-52194FE2AED0@mork.no>
Date:	Sat, 05 Mar 2016 20:53:07 +0100
From:	Bjørn Mork <bjorn@...k.no>
To:	Oliver Neukum <oneukum@...e.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	Andrey Konovalov <andreyknvl@...il.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	USB list <linux-usb@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>
Subject: Re: Possible double-free in the usbnet driver



On March 5, 2016 4:51:30 PM CET, Oliver Neukum <oneukum@...e.com> 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