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

[Hans de Goede]

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