Re: [Qemu-devel] [PATCH 17/18] usb: move cancel callback to USBDeviceInfo

[Hans de Goede <hdegoede@redhat.com>]

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