[Qemu-devel] [PATCH v4 0/3] usb: fix segfault when hot-unplugging usb host adapter

[Qemu-devel] [PATCH v4 0/3] usb: fix segfault when hot-unplugging usb host adapter

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When hot-unplugging the usb controllers (ehci/uhci),
we have to clean all resouce of these devices,
involved registered reset handler. Otherwise, it
may cause NULL pointer access and/or segmentation fault
if we reboot the guest os after hot-unplugging.
    
Let's hook up reset via DeviceClass->reset() and drop
the qemu_register_reset() call. Then Qemu will register
and unregister the reset handler automatically.

Cc: qemu-stable <qemu-stable@nongnu.org>

v4 -> v3:
 - add reset hookup for sysbus devices (ehci/ohci). (Gerd)

v2 -> v3:
 - rewrite each patch's title

v1 -> v2:
 - hooking up reset via DeviceClass->reset 
 and drop the qemu_register_reset() calls.   (Gerd)

Gonglei (3):
  uhci: fix segfault when hot-unplugging uhci controller
  ehci: fix segfault when hot-unplugging ehci controller
  ohci: fix resource cleanup leak

 hw/usb/hcd-ehci-pci.c    | 10 ++++++++++
 hw/usb/hcd-ehci-sysbus.c | 10 ++++++++++
 hw/usb/hcd-ehci.c        |  3 +--
 hw/usb/hcd-ehci.h        |  1 +
 hw/usb/hcd-ohci.c        | 20 +++++++++++++++++++-
 hw/usb/hcd-uhci.c        | 12 ++++++------
 6 files changed, 47 insertions(+), 9 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v4 1/3] uhci: fix segfault when hot-unplugging uhci controller

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When hot-unplugging the usb controllers (ehci/uhci),
we have to clean all resouce of these devices,
involved registered reset handler. Otherwise, it
may cause NULL pointer access and/or segmentation fault
if we reboot the guest os after hot-unplugging.

Let's hook up reset via DeviceClass->reset() and drop
the qemu_register_reset() call. Then Qemu will register
and unregister the reset handler automatically.

Cc: qemu-stable <qemu-stable@nongnu.org>
Reported-by: Lidonglin <lidonglin@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/hcd-uhci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index f903de7..d8f0577 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -348,9 +348,10 @@ static void uhci_update_irq(UHCIState *s)
     pci_set_irq(&s->dev, level);
 }
 
-static void uhci_reset(void *opaque)
+static void uhci_reset(DeviceState *dev)
 {
-    UHCIState *s = opaque;
+    PCIDevice *d = PCI_DEVICE(dev);
+    UHCIState *s = DO_UPCAST(UHCIState, dev, d);
     uint8_t *pci_conf;
     int i;
     UHCIPort *port;
@@ -454,11 +455,11 @@ static void uhci_port_write(void *opaque, hwaddr addr,
                 port = &s->ports[i];
                 usb_device_reset(port->port.dev);
             }
-            uhci_reset(s);
+            uhci_reset(DEVICE(s));
             return;
         }
         if (val & UHCI_CMD_HCRESET) {
-            uhci_reset(s);
+            uhci_reset(DEVICE(s));
             return;
         }
         s->cmd = val;
@@ -1226,8 +1227,6 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
     s->num_ports_vmstate = NB_PORTS;
     QTAILQ_INIT(&s->queues);
 
-    qemu_register_reset(uhci_reset, s);
-
     memory_region_init_io(&s->io_bar, OBJECT(s), &uhci_ioport_ops, s,
                           "uhci", 0x20);
 
@@ -1303,6 +1302,7 @@ static void uhci_class_init(ObjectClass *klass, void *data)
     k->revision  = info->revision;
     k->class_id  = PCI_CLASS_SERIAL_USB;
     dc->vmsd = &vmstate_uhci;
+    dc->reset = uhci_reset;
     if (!info->unplug) {
         /* uhci controllers in companion setups can't be hotplugged */
         dc->hotpluggable = false;
-- 
1.7.12.4

[Qemu-devel] [PATCH v4 2/3] ehci: fix segfault when hot-unplugging ehci controller

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When hot-unplugging the usb controllers (ehci/uhci),
we have to clean all resouce of these devices,
involved registered reset handler. Otherwise, it
may cause NULL pointer access and/or segmentation fault
if we reboot the guest os after hot-unplugging.

Let's hook up reset via DeviceClass->reset() and drop
the qemu_register_reset() call. Then Qemu will register
and unregister the reset handler automatically.

Cc: qemu-stable <qemu-stable@nongnu.org>
Reported-by: Lidonglin <lidonglin@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/hcd-ehci-pci.c    | 10 ++++++++++
 hw/usb/hcd-ehci-sysbus.c | 10 ++++++++++
 hw/usb/hcd-ehci.c        |  3 +--
 hw/usb/hcd-ehci.h        |  1 +
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 4c80707..7afa5f9 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -101,6 +101,15 @@ static void usb_ehci_pci_exit(PCIDevice *dev)
     }
 }
 
+static void usb_ehci_pci_reset(DeviceState *dev)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(dev);
+    EHCIPCIState *i = PCI_EHCI(pci_dev);
+    EHCIState *s = &i->ehci;
+
+    ehci_reset(s);
+}
+
 static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
                                       uint32_t val, int l)
 {
@@ -143,6 +152,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
     k->config_write = usb_ehci_pci_write_config;
     dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_pci_properties;
+    dc->reset = usb_ehci_pci_reset;
 }
 
 static const TypeInfo ehci_pci_type_info = {
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 19ed2c2..cd1cc14 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -42,6 +42,15 @@ static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
     sysbus_init_irq(d, &s->irq);
 }
 
+static void usb_ehci_sysbus_reset(DeviceState *dev)
+{
+    SysBusDevice *d = SYS_BUS_DEVICE(dev);
+    EHCISysBusState *i = SYS_BUS_EHCI(d);
+    EHCIState *s = &i->ehci;
+
+    ehci_reset(s);
+}
+
 static void ehci_sysbus_init(Object *obj)
 {
     SysBusDevice *d = SYS_BUS_DEVICE(obj);
@@ -70,6 +79,7 @@ static void ehci_sysbus_class_init(ObjectClass *klass, void *data)
     dc->realize = usb_ehci_sysbus_realize;
     dc->vmsd = &vmstate_ehci_sysbus;
     dc->props = ehci_sysbus_properties;
+    dc->reset = usb_ehci_sysbus_reset;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index ccf54b6..2a84f46 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -845,7 +845,7 @@ static USBDevice *ehci_find_device(EHCIState *ehci, uint8_t addr)
 }
 
 /* 4.1 host controller initialization */
-static void ehci_reset(void *opaque)
+void ehci_reset(void *opaque)
 {
     EHCIState *s = opaque;
     int i;
@@ -2471,7 +2471,6 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp)
     s->async_bh = qemu_bh_new(ehci_frame_timer, s);
     s->device = dev;
 
-    qemu_register_reset(ehci_reset, s);
     s->vmstate = qemu_add_vm_change_state_handler(usb_ehci_vm_state_change, s);
 }
 
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 2bc259c..87b240f 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -325,6 +325,7 @@ extern const VMStateDescription vmstate_ehci;
 void usb_ehci_init(EHCIState *s, DeviceState *dev);
 void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp);
 void usb_ehci_unrealize(EHCIState *s, DeviceState *dev, Error **errp);
+void ehci_reset(void *opaque);
 
 #define TYPE_PCI_EHCI "pci-ehci-usb"
 #define PCI_EHCI(obj) OBJECT_CHECK(EHCIPCIState, (obj), TYPE_PCI_EHCI)
-- 
1.7.12.4

[Qemu-devel] [PATCH v4 3/3] ohci: fix resource cleanup leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When hot-unplugging the usb controllers (ehci/uhci),
we have to clean all resouce of these devices,
involved registered reset handler. Otherwise, it
may cause NULL pointer access and/or segmentation fault
if we reboot the guest os after hot-unplugging.

Let's hook up reset via DeviceClass->reset() and drop
the qemu_register_reset() call. Then Qemu will register
and unregister the reset handler automatically.

Ohci does't support hotplugging/hotunplugging yet, but
existing resource cleanup leak logic likes ehci/uhci.

Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/hcd-ohci.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index a0d478e..ca7aeb3 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1879,7 +1879,6 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
     usb_packet_init(&ohci->usb_packet);
 
     ohci->async_td = 0;
-    qemu_register_reset(ohci_reset, ohci);
 
     return 0;
 }
@@ -1951,6 +1950,15 @@ static void usb_ohci_exit(PCIDevice *dev)
     }
 }
 
+static void usb_ohci_reset_pci(DeviceState *d)
+{
+    PCIDevice *dev = PCI_DEVICE(d);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    ohci_reset(s);
+}
+
 #define TYPE_SYSBUS_OHCI "sysbus-ohci"
 #define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
 
@@ -1976,6 +1984,14 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
     sysbus_init_mmio(sbd, &s->ohci.mem);
 }
 
+static void usb_ohci_reset_sysbus(DeviceState *dev)
+{
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
+    OHCIState *ohci = &s->ohci;
+
+    ohci_reset(ohci);
+}
+
 static Property ohci_pci_properties[] = {
     DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
@@ -2097,6 +2113,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
     dc->props = ohci_pci_properties;
     dc->hotpluggable = false;
     dc->vmsd = &vmstate_ohci;
+    dc->reset = usb_ohci_reset_pci;
 }
 
 static const TypeInfo ohci_pci_info = {
@@ -2120,6 +2137,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     dc->desc = "OHCI USB Controller";
     dc->props = ohci_sysbus_properties;
+    dc->reset = usb_ohci_reset_sysbus;
 }
 
 static const TypeInfo ohci_sysbus_info = {
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v4 0/3] usb: fix segfault when hot-unplugging usb host adapter

[Gerd Hoffmann]

On Mi, 2015-03-18 at 17:33 +0800, arei.gonglei@huawei.com wrote:
> v4 -> v3:
>  - add reset hookup for sysbus devices (ehci/ohci). (Gerd)

Replaced patches.  make check failure is gone.  Good.

thanks,
  Gerd