Re: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On 08/23/2013 05:07 PM, Alan Stern wrote: On Fri, 23 Aug 2013, Xenia Ragiadakou wrote: Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? Yes. I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. usb_reset_and_verify_device() checks to see if the descriptors changed when the device was reset. (If they did then we need to re-enumerate the device.) It calls descriptors_changed() to do this checking. Originally there were no BOS descriptors, so of course the routine didn't try to compare them. But now that they exist, we need to compare the old and new descriptor values, same as for the device and config descriptors. And of course we need to deallocate one of the BOS descriptors, instead of leaking it. Alan Stern Thanks for the clarification. I will send a patch as RFC because i am not quite familiar and because i have a problem to run kmemleak effectively (i can only scan for a few seconds, although i have set all necessary kmemleak configuration, i have not figured out why yet). I perform the deallocation outside the descriptors_changed(). That leads to code duplication, but if i perform it inside, the function will do something that is not expected to do, right? Also, another thing is that even if the device descriptors have not change (and thus the bcdUSB), there is still the possibility one of the two bos descriptors to be null while the other one is not, either due to a kmalloc() failure or in case the usb device version is in the range [0x201, 0x210) where bos descriptors are optional and if new firmware is downloaded in the device may has as result to add or remove the bos descriptor support. Am i thinking correctly here? regards, ksenia -- 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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Mon, 26 Aug 2013, Xenia Ragiadakou wrote: I will send a patch as RFC because i am not quite familiar and because i have a problem to run kmemleak effectively (i can only scan for a few seconds, although i have set all necessary kmemleak configuration, i have not figured out why yet). I perform the deallocation outside the descriptors_changed(). That leads to code duplication, but if i perform it inside, the function will do something that is not expected to do, right? Right. Also, another thing is that even if the device descriptors have not change (and thus the bcdUSB), there is still the possibility one of the two bos descriptors to be null while the other one is not, either due to a kmalloc() failure or in case the usb device version is in the range [0x201, 0x210) where bos descriptors are optional and if new firmware is downloaded in the device may has as result to add or remove the bos descriptor support. Am i thinking correctly here? That's right. Any difference at all should cause the device to be re-enumerated. Alan Stern -- 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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On 08/14/2013 08:24 AM, Sarah Sharp wrote: On Fri, Aug 09, 2013 at 05:14:48PM -0400, Alan Stern wrote: On Fri, 9 Aug 2013, Greg KH wrote: On Thu, Aug 08, 2013 at 10:07:19PM +0200, Martin MOKREJŠ wrote: Hi, I get plenty of these in /sys/kernel/debug/kmemleak: unreferenced object 0x88019f675268 (size 32): comm usb-storage, pid 11411, jiffies 4310515592 (age 1538.100s) hex dump (first 32 bytes): 05 0f 16 00 02 07 10 02 02 00 00 00 0a 10 03 00 0e 00 01 0a ff 07 00 00 00 00 00 00 00 00 00 00 backtrace: [81811ca1] kmemleak_alloc+0x21/0x50 [81166c5e] __kmalloc+0xce/0x170 [815270d5] usb_get_bos_descriptor+0xc5/0x230 [81519048] hub_port_init+0x768/0xa20 [81519416] usb_reset_and_verify_device+0xd6/0x540 [81519990] usb_reset_device+0x110/0x190 [8154aed4] usb_stor_port_reset+0x74/0x80 [8154af6f] usb_stor_invoke_transport+0x8f/0x550 [81549df9] usb_stor_transparent_scsi_command+0x9/0x10 [8154b7ab] usb_stor_control_thread+0x16b/0x280 [810b8c95] kthread+0xe5/0xf0 [8182caec] ret_from_fork+0x7c/0xb0 [] 0x Odd, Andiry and Sarah, any thoughts? dmesg left below for completeness. Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. best regards, ksenia -- 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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Fri, 23 Aug 2013, Xenia Ragiadakou wrote: Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Sarah Sharp Hi, I will try to fix this leak :) First i would like to ensure that i understood the problem so ... If i understood well the usb device's bos descriptor is allocated everytime hub_port_init() is called without never being freed, right? Yes. I do not understand why to compare the old bos with the new one, and not just release the old one or store the new in the memory allocated for the old one. usb_reset_and_verify_device() checks to see if the descriptors changed when the device was reset. (If they did then we need to re-enumerate the device.) It calls descriptors_changed() to do this checking. Originally there were no BOS descriptors, so of course the routine didn't try to compare them. But now that they exist, we need to compare the old and new descriptor values, same as for the device and config descriptors. And of course we need to deallocate one of the BOS descriptors, instead of leaking it. Alan Stern -- 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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Tue, 13 Aug 2013, Sarah Sharp wrote: Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. I'll put it in my list of future tasks to give to Xenia. :) Alan, if you have any other small tasks for OPW interns, please let me know. Good idea. I'll keep it in mind. Alan Stern -- 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: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Thu, Aug 08, 2013 at 10:07:19PM +0200, Martin MOKREJŠ wrote: Hi, I get plenty of these in /sys/kernel/debug/kmemleak: unreferenced object 0x88019f675268 (size 32): comm usb-storage, pid 11411, jiffies 4310515592 (age 1538.100s) hex dump (first 32 bytes): 05 0f 16 00 02 07 10 02 02 00 00 00 0a 10 03 00 0e 00 01 0a ff 07 00 00 00 00 00 00 00 00 00 00 backtrace: [81811ca1] kmemleak_alloc+0x21/0x50 [81166c5e] __kmalloc+0xce/0x170 [815270d5] usb_get_bos_descriptor+0xc5/0x230 [81519048] hub_port_init+0x768/0xa20 [81519416] usb_reset_and_verify_device+0xd6/0x540 [81519990] usb_reset_device+0x110/0x190 [8154aed4] usb_stor_port_reset+0x74/0x80 [8154af6f] usb_stor_invoke_transport+0x8f/0x550 [81549df9] usb_stor_transparent_scsi_command+0x9/0x10 [8154b7ab] usb_stor_control_thread+0x16b/0x280 [810b8c95] kthread+0xe5/0xf0 [8182caec] ret_from_fork+0x7c/0xb0 [] 0x Odd, Andiry and Sarah, any thoughts? dmesg left below for completeness. greg k-h I believe these lines from dmesg are relevant: [4.201374] usb 2-1: New USB device found, idVendor=8087, idProduct=0024 [4.202334] usb 2-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [4.205214] hub 2-1:1.0: USB hub found [4.206389] hub 2-1:1.0: 8 ports detected [4.330926] usb 3-1: new high-speed USB device number 2 using xhci_hcd [4.354303] usb 3-1: New USB device found, idVendor=2109, idProduct=3431 [4.356636] usb 3-1: New USB device strings: Mfr=0, Product=1, SerialNumber=0 [4.358924] usb 3-1: Product: USB2.0 Hub [4.387716] hub 3-1:1.0: USB hub found [4.38] hub 3-1:1.0: 4 ports detected [4.521136] usb 3-2: new high-speed USB device number 3 using xhci_hcd [4.541084] ata6: SATA link up 3.0 Gbps (SStatus 123 SControl 300) [4.544025] ata6.00: ATA-8: ST3000VX000-1CU166, CV22, max UDMA/133 [4.545766] ata6.00: 5860533168 sectors, multi 16: LBA48 NCQ (depth 31/32), AA [4.547652] usb 3-2: New USB device found, idVendor=2109, idProduct=0811 [4.549396] usb 3-2: New USB device strings: Mfr=0, Product=1, SerialNumber=0 [4.551184] usb 3-2: Product: USB2.0 Hub [4.553356] ata6.00: configured for UDMA/133 [4.555444] scsi 5:0:0:0: Direct-Access ATA ST3000VX000-1CU1 CV22 PQ: 0 ANSI: 5 [4.560417] sd 5:0:0:0: [sdb] 5860533168 512-byte logical blocks: (3.00 TB/2.72 TiB) [4.561809] sd 5:0:0:0: [sdb] 4096-byte physical blocks [4.563824] sd 5:0:0:0: [sdb] Write Protect is off [4.565122] sd 5:0:0:0: [sdb] Mode Sense: 00 3a 00 00 [4.567672] sd 5:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [4.572740] sd 5:0:0:0: Attached scsi generic sg2 type 0 [4.578793] hub 3-2:1.0: USB hub found [4.580040] hub 3-2:1.0: 4 ports detected [4.628775] sdb: sdb1 [4.631812] sd 5:0:0:0: [sdb] Attached SCSI disk [4.712068] usb 4-2: new SuperSpeed USB device number 2 using xhci_hcd [4.733347] usb 4-2: Parent hub missing LPM exit latency info. Power management will be impacted. [4.738124] usb 4-2: New USB device found, idVendor=2109, idProduct=0811 [4.739477] usb 4-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [4.740835] usb 4-2: Product: 4-Port USB 3.0 Hub [4.742179] usb 4-2: Manufacturer: VIA Labs, Inc. [4.771771] hub 4-2:1.0: USB hub found [4.773737] hub 4-2:1.0: 4 ports detected [158794.376525] usb 3-1.3: reset high-speed USB device number 9 using xhci_hcd [158794.581028] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 88018d012da8 [158794.581033] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 88018d012de8 [158829.439903] usb 3-1.3: reset high-speed USB device number 9 using xhci_hcd [158829.514208] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 88018d012da8 [158829.514213] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with disabled ep 88018d012de8 [159013.517275] CPU1: Core temperature above threshold, cpu clock throttled (total events = 3846484) [159013.517278] CPU0: Package temperature above threshold, cpu clock throttled (total events = 4139411) [159013.517283] CPU1: Package temperature above threshold, cpu clock throttled (total events = 4139471) [159013.518287] CPU1: Core temperature/speed normal [159013.518288] CPU0: Package temperature/speed normal [159013.518291] CPU1: Package temperature/speed normal [159221.397277] kmemleak: 16 new suspected memory leaks (see /sys/kernel/debug/kmemleak) [159256.419270] usb 3-1.3: reset high-speed USB device number 9 using xhci_hcd [159256.493409] xhci_hcd :0b:00.0: xHCI xhci_drop_endpoint called with
Re: 3.10.4: kmemleak in usb_get_bos_descriptor()?
On Fri, 9 Aug 2013, Greg KH wrote: On Thu, Aug 08, 2013 at 10:07:19PM +0200, Martin MOKREJŠ wrote: Hi, I get plenty of these in /sys/kernel/debug/kmemleak: unreferenced object 0x88019f675268 (size 32): comm usb-storage, pid 11411, jiffies 4310515592 (age 1538.100s) hex dump (first 32 bytes): 05 0f 16 00 02 07 10 02 02 00 00 00 0a 10 03 00 0e 00 01 0a ff 07 00 00 00 00 00 00 00 00 00 00 backtrace: [81811ca1] kmemleak_alloc+0x21/0x50 [81166c5e] __kmalloc+0xce/0x170 [815270d5] usb_get_bos_descriptor+0xc5/0x230 [81519048] hub_port_init+0x768/0xa20 [81519416] usb_reset_and_verify_device+0xd6/0x540 [81519990] usb_reset_device+0x110/0x190 [8154aed4] usb_stor_port_reset+0x74/0x80 [8154af6f] usb_stor_invoke_transport+0x8f/0x550 [81549df9] usb_stor_transparent_scsi_command+0x9/0x10 [8154b7ab] usb_stor_control_thread+0x16b/0x280 [810b8c95] kthread+0xe5/0xf0 [8182caec] ret_from_fork+0x7c/0xb0 [] 0x Odd, Andiry and Sarah, any thoughts? dmesg left below for completeness. Martin is right; the BOS descriptors are leaked in usb_reset_and_verify_device(). We need to store the old descriptor, compare it with the new one following the reset, and delete one of them afterward. I haven't had time to fix this. Alan Stern -- 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
