From a6e037390dd91276f4a631d41188c87e8a60bb3f Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Sun, 8 Nov 2020 17:33:50 +1100 Subject: [PATCH 1/6] audio/jack: fix use after free segfault This change registers a bottom handler to close the JACK client connection when a server shutdown signal is received. Without this libjack2 attempts to "clean up" old clients and causes a use after free segfault. Signed-off-by: Geoffrey McRae Reviewed-by: Christian Schoenebeck Message-Id: <20201108063351.35804-2-geoff@hostfission.com> Signed-off-by: Gerd Hoffmann --- audio/jackaudio.c | 50 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/audio/jackaudio.c b/audio/jackaudio.c index 1e714b30bc..3b7c18443d 100644 --- a/audio/jackaudio.c +++ b/audio/jackaudio.c @@ -25,6 +25,7 @@ #include "qemu/osdep.h" #include "qemu/module.h" #include "qemu/atomic.h" +#include "qemu/main-loop.h" #include "qemu-common.h" #include "audio.h" @@ -63,6 +64,7 @@ typedef struct QJackClient { QJackState state; jack_client_t *client; jack_nframes_t freq; + QEMUBH *shutdown_bh; struct QJack *j; int nchannels; @@ -87,6 +89,7 @@ QJackIn; static int qjack_client_init(QJackClient *c); static void qjack_client_connect_ports(QJackClient *c); static void qjack_client_fini(QJackClient *c); +static QemuMutex qjack_shutdown_lock; static void qjack_buffer_create(QJackBuffer *buffer, int channels, int frames) { @@ -306,21 +309,27 @@ static int qjack_xrun(void *arg) return 0; } +static void qjack_shutdown_bh(void *opaque) +{ + QJackClient *c = (QJackClient *)opaque; + qjack_client_fini(c); +} + static void qjack_shutdown(void *arg) { QJackClient *c = (QJackClient *)arg; c->state = QJACK_STATE_SHUTDOWN; + qemu_bh_schedule(c->shutdown_bh); } static void qjack_client_recover(QJackClient *c) { - if (c->state == QJACK_STATE_SHUTDOWN) { - qjack_client_fini(c); + if (c->state != QJACK_STATE_DISCONNECTED) { + return; } /* packets is used simply to throttle this */ - if (c->state == QJACK_STATE_DISCONNECTED && - c->packets % 100 == 0) { + if (c->packets % 100 == 0) { /* if enabled then attempt to recover */ if (c->enabled) { @@ -489,15 +498,16 @@ static int qjack_init_out(HWVoiceOut *hw, struct audsettings *as, QJackOut *jo = (QJackOut *)hw; Audiodev *dev = (Audiodev *)drv_opaque; - qjack_client_fini(&jo->c); - jo->c.out = true; jo->c.enabled = false; jo->c.nchannels = as->nchannels; jo->c.opt = dev->u.jack.out; + jo->c.shutdown_bh = qemu_bh_new(qjack_shutdown_bh, &jo->c); + int ret = qjack_client_init(&jo->c); if (ret != 0) { + qemu_bh_delete(jo->c.shutdown_bh); return ret; } @@ -525,15 +535,16 @@ static int qjack_init_in(HWVoiceIn *hw, struct audsettings *as, QJackIn *ji = (QJackIn *)hw; Audiodev *dev = (Audiodev *)drv_opaque; - qjack_client_fini(&ji->c); - ji->c.out = false; ji->c.enabled = false; ji->c.nchannels = as->nchannels; ji->c.opt = dev->u.jack.in; + ji->c.shutdown_bh = qemu_bh_new(qjack_shutdown_bh, &ji->c); + int ret = qjack_client_init(&ji->c); if (ret != 0) { + qemu_bh_delete(ji->c.shutdown_bh); return ret; } @@ -555,7 +566,7 @@ static int qjack_init_in(HWVoiceIn *hw, struct audsettings *as, return 0; } -static void qjack_client_fini(QJackClient *c) +static void qjack_client_fini_locked(QJackClient *c) { switch (c->state) { case QJACK_STATE_RUNNING: @@ -564,28 +575,40 @@ static void qjack_client_fini(QJackClient *c) case QJACK_STATE_SHUTDOWN: jack_client_close(c->client); + c->client = NULL; + + qjack_buffer_free(&c->fifo); + g_free(c->port); + + c->state = QJACK_STATE_DISCONNECTED; /* fallthrough */ case QJACK_STATE_DISCONNECTED: break; } +} - qjack_buffer_free(&c->fifo); - g_free(c->port); - - c->state = QJACK_STATE_DISCONNECTED; +static void qjack_client_fini(QJackClient *c) +{ + qemu_mutex_lock(&qjack_shutdown_lock); + qjack_client_fini_locked(c); + qemu_mutex_unlock(&qjack_shutdown_lock); } static void qjack_fini_out(HWVoiceOut *hw) { QJackOut *jo = (QJackOut *)hw; qjack_client_fini(&jo->c); + + qemu_bh_delete(jo->c.shutdown_bh); } static void qjack_fini_in(HWVoiceIn *hw) { QJackIn *ji = (QJackIn *)hw; qjack_client_fini(&ji->c); + + qemu_bh_delete(ji->c.shutdown_bh); } static void qjack_enable_out(HWVoiceOut *hw, bool enable) @@ -662,6 +685,7 @@ static void qjack_info(const char *msg) static void register_audio_jack(void) { + qemu_mutex_init(&qjack_shutdown_lock); audio_driver_register(&jack_driver); jack_set_thread_creator(qjack_thread_creator); jack_set_error_function(qjack_error); From 1cd8b9487025966123287e532636f231b46e8398 Mon Sep 17 00:00:00 2001 From: lichun Date: Sat, 7 Nov 2020 01:03:39 +0800 Subject: [PATCH 2/6] console: avoid passing con=NULL to graphic_hw_update_done() In graphic_hw_update(), first select an existing console, a specific-console or active_console(if not specified), then updating the console. Signed-off-by: lichun Message-id: 1604682219-114389-1-git-send-email-lichun@ruijie.com.cn Signed-off-by: Gerd Hoffmann --- ui/console.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/console.c b/ui/console.c index e8e59707d3..e07d2c380d 100644 --- a/ui/console.c +++ b/ui/console.c @@ -270,10 +270,11 @@ void graphic_hw_update_done(QemuConsole *con) void graphic_hw_update(QemuConsole *con) { bool async = false; + con = con ? con : active_console; if (!con) { - con = active_console; + return; } - if (con && con->hw_ops->gfx_update) { + if (con->hw_ops->gfx_update) { con->hw_ops->gfx_update(con->hw); async = con->hw_ops->gfx_update_async; } From f0617abfd6c08a7711cb99cc02a74533846c2f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 9 Nov 2020 14:52:57 +0100 Subject: [PATCH 3/6] hw/usb/Kconfig: Fix USB_XHCI_NEC (depends on USB_XHCI_PCI) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 755fba11fbc and 8ddab8dd3d8 we can not build USB_XHCI_NEC without USB_XHCI_PCI. Correct the Kconfig dependency. Fixes: 755fba11fbc ("usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c") Reviewed-by: Sai Pavan Boddu Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé Message-id: 20201109135300.2592982-2-philmd@redhat.com [ kraxel: restore "default y if PCI_DEVICES" because "qemu-system-ppc64 -M pseries,usb=on" needs USB_XHCI_NEC=y ] Signed-off-by: Gerd Hoffmann --- hw/usb/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig index a674ce4c54..3b07d9cf68 100644 --- a/hw/usb/Kconfig +++ b/hw/usb/Kconfig @@ -43,8 +43,7 @@ config USB_XHCI_PCI config USB_XHCI_NEC bool default y if PCI_DEVICES - depends on PCI - select USB_XHCI + select USB_XHCI_PCI config USB_XHCI_SYSBUS bool From 0d5528612badad84821c5d93c82253f86987659d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 3 Nov 2020 12:25:55 +0100 Subject: [PATCH 4/6] hw/display/cirrus_vga: Remove debugging code commented out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit ec87f206d70 ("cirrus: replace debug printf with trace points") forgot to remove this code once replaced. Do it now. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Dr. David Alan Gilbert Message-id: 20201103112558.2554390-2-philmd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/cirrus_vga.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index 722b9e7004..e14096deb4 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2532,9 +2532,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr, case 0x3c5: val = cirrus_vga_read_sr(c); break; -#ifdef DEBUG_VGA_REG - printf("vga: read SR%x = 0x%02x\n", s->sr_index, val); -#endif break; case 0x3c6: val = cirrus_read_hidden_dac(c); @@ -2560,9 +2557,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr, break; case 0x3cf: val = cirrus_vga_read_gr(c, s->gr_index); -#ifdef DEBUG_VGA_REG - printf("vga: read GR%x = 0x%02x\n", s->gr_index, val); -#endif break; case 0x3b4: case 0x3d4: @@ -2571,9 +2565,6 @@ static uint64_t cirrus_vga_ioport_read(void *opaque, hwaddr addr, case 0x3b5: case 0x3d5: val = cirrus_vga_read_cr(c, s->cr_index); -#ifdef DEBUG_VGA_REG - printf("vga: read CR%x = 0x%02x\n", s->cr_index, val); -#endif break; case 0x3ba: case 0x3da: @@ -2645,9 +2636,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val, s->sr_index = val; break; case 0x3c5: -#ifdef DEBUG_VGA_REG - printf("vga: write SR%x = 0x%02" PRIu64 "\n", s->sr_index, val); -#endif cirrus_vga_write_sr(c, val); break; case 0x3c6: @@ -2670,9 +2658,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val, s->gr_index = val; break; case 0x3cf: -#ifdef DEBUG_VGA_REG - printf("vga: write GR%x = 0x%02" PRIu64 "\n", s->gr_index, val); -#endif cirrus_vga_write_gr(c, s->gr_index, val); break; case 0x3b4: @@ -2681,9 +2666,6 @@ static void cirrus_vga_ioport_write(void *opaque, hwaddr addr, uint64_t val, break; case 0x3b5: case 0x3d5: -#ifdef DEBUG_VGA_REG - printf("vga: write CR%x = 0x%02"PRIu64"\n", s->cr_index, val); -#endif cirrus_vga_write_cr(c, val); break; case 0x3ba: From e016a844ddfc94167fa25d55b46e1d1accafdec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 3 Nov 2020 12:25:56 +0100 Subject: [PATCH 5/6] hw/display/cirrus_vga: Fix hexadecimal format string specifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The '%u' conversion specifier is for decimal notation. When prefixing a format with '0x', we want the hexadecimal specifier ('%x'). Inspired-by: Dov Murik Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Dr. David Alan Gilbert Message-id: 20201103112558.2554390-3-philmd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/cirrus_vga.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index e14096deb4..fdca6ca659 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2105,7 +2105,7 @@ static void cirrus_vga_mem_write(void *opaque, } else { qemu_log_mask(LOG_GUEST_ERROR, "cirrus: mem_writeb 0x" TARGET_FMT_plx " " - "value 0x%02" PRIu64 "\n", addr, mem_value); + "value 0x%02" PRIx64 "\n", addr, mem_value); } } From 172bc8520db1cb98d09b367360068a675fbc9413 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 12 Nov 2020 11:37:41 +0100 Subject: [PATCH 6/6] xhci: fix nec-usb-xhci properties Storing properties directly in XHCIPciState.xhci doesn't work, the object_initialize_child() call in xhci_instance_init() will overwrite them. This changes the defaults for some properties, which in turn breaks live migration and possibly other things as well. So add XHCINecState, store properties there, copy them over on instance init. Fixes: 8ddab8dd3d81 ("usb/hcd-xhci: Split pci wrapper for xhci base model") Reported-by: Dr. David Alan Gilbert Signed-off-by: Gerd Hoffmann Message-id: 20201112103741.2335-1-kraxel@redhat.com --- hw/usb/hcd-xhci-nec.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/hw/usb/hcd-xhci-nec.c b/hw/usb/hcd-xhci-nec.c index 5707b2cabd..13a125afe2 100644 --- a/hw/usb/hcd-xhci-nec.c +++ b/hw/usb/hcd-xhci-nec.c @@ -27,18 +27,37 @@ #include "hcd-xhci-pci.h" +typedef struct XHCINecState { + /*< private >*/ + XHCIPciState parent_obj; + /*< public >*/ + uint32_t flags; + uint32_t intrs; + uint32_t slots; +} XHCINecState; + static Property nec_xhci_properties[] = { DEFINE_PROP_ON_OFF_AUTO("msi", XHCIPciState, msi, ON_OFF_AUTO_AUTO), DEFINE_PROP_ON_OFF_AUTO("msix", XHCIPciState, msix, ON_OFF_AUTO_AUTO), - DEFINE_PROP_BIT("superspeed-ports-first", XHCIPciState, - xhci.flags, XHCI_FLAG_SS_FIRST, true), - DEFINE_PROP_BIT("force-pcie-endcap", XHCIPciState, xhci.flags, + DEFINE_PROP_BIT("superspeed-ports-first", XHCINecState, flags, + XHCI_FLAG_SS_FIRST, true), + DEFINE_PROP_BIT("force-pcie-endcap", XHCINecState, flags, XHCI_FLAG_FORCE_PCIE_ENDCAP, false), - DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs, XHCI_MAXINTRS), - DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots, XHCI_MAXSLOTS), + DEFINE_PROP_UINT32("intrs", XHCINecState, intrs, XHCI_MAXINTRS), + DEFINE_PROP_UINT32("slots", XHCINecState, slots, XHCI_MAXSLOTS), DEFINE_PROP_END_OF_LIST(), }; +static void nec_xhci_instance_init(Object *obj) +{ + XHCIPciState *pci = XHCI_PCI(obj); + XHCINecState *nec = container_of(pci, XHCINecState, parent_obj); + + pci->xhci.flags = nec->flags; + pci->xhci.numintrs = nec->intrs; + pci->xhci.numslots = nec->slots; +} + static void nec_xhci_class_init(ObjectClass *klass, void *data) { PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); @@ -53,6 +72,8 @@ static void nec_xhci_class_init(ObjectClass *klass, void *data) static const TypeInfo nec_xhci_info = { .name = TYPE_NEC_XHCI, .parent = TYPE_XHCI_PCI, + .instance_size = sizeof(XHCINecState), + .instance_init = nec_xhci_instance_init, .class_init = nec_xhci_class_init, };