linux-kernel - Re: kmemleak report on isp1763 and sierra MC8705

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: <CAMgPeKUPeJSKas2VOqbG+Nt5MOtdA3jUUCsFwUcDVhwDJJCr6A@mail.gmail.com>
Date:	Sat, 10 Nov 2012 15:30:10 +0100
From:	Johan Hovold <jhovold@...il.com>
To:	Richard Retanubun <richardretanubun@...gedcom.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Lennart Sorensen <lsorense@...lub.uwaterloo.ca>,
	Tang Nguyen <TangNguyen@...gedcom.com>,
	linux-usb mailing list <linux-usb@...r.kernel.org>,
	Johan Hovold <jhovold@...il.com>
Subject: Re: kmemleak report on isp1763 and sierra MC8705

On Fri, Nov 9, 2012 at 11:14 PM, Richard Retanubun
<richardretanubun@...gedcom.com> wrote:
> On 29/10/12 06:14 PM, Alan Stern wrote:
>>
>> On Mon, 29 Oct 2012, Richard Retanubun wrote:
>>>
>>> Focusing down on one of the dumps:
>>>
>>> unreferenced object 0xd3849740 (size 8):
>>>     comm "khubd", pid 1026, jiffies 232553037 (age 506.597s)
>>>     hex dump (first 8 bytes):
>>>       4d 43 38 37 30 35 00 00                          MC8705..
>>>     backtrace:
>>>       [<e30efd74>] usb_cache_string+0x74/0xac [usbcore]
>>>       [<e30e77bc>] usb_enumerate_device+0x44/0xf8 [usbcore]
>>>       [<e30e7aa0>] usb_new_device+0x3c/0x13c [usbcore]
>>>       [<e30e9824>] hub_thread+0xc8c/0x1544 [usbcore]
>>>       [<c0043aa8>] kthread+0x7c/0x80
>>>       [<c000ed48>] kernel_thread+0x4c/0x68
>>>
>>> I have a small question. How does the memory kmalloc-ed() in
>>> usb_cache_string is supposed to be released?
>>> (during usb_serial_disconnect()?)
>>
>>
>> It doesn't get released during usb_serial_disconnect().  It gets
>> released during usb_release_dev() in drivers/usb/core/usb.c.
>>
>>>   Is the sierra driver is supposed to participate
>>> in the tear down process (in sierra_release() maybe) and not doing
>>> something that is expected?
>>
>>
>> Probably not.
>>
>>> I am still missing the link between the actions done by the hub_thread()
>>> for the caching the stings
>>> and the sierra driver code.
>>
>>
>> They aren't all that closely related.
>>
>> usb_release_dev() won't be called until all references to the USB
>> device have been dropped.  Maybe there's an extra reference hanging
>> around.
>>
>> Alan Stern
>>
> Thanks a lot for the hint Alan.
>
> I added a dev_dbg print in usb_release_dev() and saw that in the builds
> where there is a leak, this was simply never called!
> the last line printed in a trace with all dev_dbg on is this
> "usb_disable_device nuking all URBs"
> When the sierra modem is unplugged, the cleanup sequence never calls
> usb_release_dev() (on PL2303 it always calls usb_release_dev()
>
> This is the current state of versions from linux-stable
>
> 3.0.y (3.0.51) - Have the issue
> 3.2.y (3.2.33) - Have the issue
> 3.4.y (3.4.18) - Have the issue
> 3.5.y (3.5.7)  - Does not have the issue (but leaks because the portdata
> patches is not backported yet)
> 3.6.y (3.6.6)  - Does not have the issue
>
> So a diff between 3.4.y and 3.5.y ought to narrow it down further.
>
> I am posting just in case someone recalls a particular patch I should be
> trying out first...

There was a reference-count fix for the probe error path that went in to
v3.5. Haven't read all the details on how you trigger your leak, but at
the face of it, it could be related.

Have a look at 0658a3366db7e27fa ("usb: use usb_serial_put in
usb_serial_probe errors). If related, you should be seeing "Ignoring
blacklisted interface #n" messages when you enable debug (e.g. #define
DEBUG) in the sierra driver.

Greg, it seems to me that the fix referred to above should be backported
to the earlier stable trees either way.

Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/