|
From: | Hans de Goede |
Subject: | Re: [Qemu-devel] [PATCH 17/18] usb: move cancel callback to USBDeviceInfo |
Date: | Mon, 23 May 2011 19:30:06 +0200 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc15 Lightning/1.0b2 Thunderbird/3.1.10 |
Hi, On 05/23/2011 04:34 PM, Gerd Hoffmann wrote:
Hi,The problem is that the USBDevice lifetime may be shorter then the USBPacket lifetime, USBPackets are created by uhci.c (for example), where as the device is managed from the monitor (for example), doing a usb_del in the monitor using the guest bus:addr will call usb_device_delete_addr, which will call qdev_free. At this time the USBDevice struct is gone, and at a later time the uhci code will cancel any still outstanding async packets, who's owner pointer will now point to free-ed memory.Good spotting, this is indeed a issue which needs fixing. It isn't introduced by this patch though, it exists even without the usb patch queue. usb-msd.c passes a USBDevice pointer directly as opaque. The usb-linux.c callback function assumes it can dereference aurb->hdev just fine.
Ah, that is no good, my usb network redir device code uses aurb's similar too linux.c, but on device-destroy walks its list of pending aurbs, sends a cancel to the host-os, and sets aurb->hdev to null, and the async cancel checks for aurb->hdev being NULL and in that case only frees the aurb and does nothing else. > Both will hit free'ed memory in case the device is unplugged while a async packet is in flight. Yep, linux.c could be fixed the same way as my usb net redir device code. But I like the patch you just send better. It looks incomplete though, I'll give more details in a reply to the patch it self. Regards, Hans
[Prev in Thread] | Current Thread | [Next in Thread] |