From 6a0fe9263d0369fbc8e76bf311db9bb4aad21576 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 3 Apr 2013 11:29:31 +0200 Subject: [PATCH] Fix use after free in ehci code (bz #890320) --- ...Don-t-access-packet-after-freeing-it.patch | 40 +++++++++++++++ ...addr-after-cancelling-an-already-com.patch | 49 +++++++++++++++++++ qemu.spec | 11 ++++- 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 0718-ehci-Don-t-access-packet-after-freeing-it.patch create mode 100644 0719-ehci-Fixup-q-qtdaddr-after-cancelling-an-already-com.patch diff --git a/0718-ehci-Don-t-access-packet-after-freeing-it.patch b/0718-ehci-Don-t-access-packet-after-freeing-it.patch new file mode 100644 index 0000000..1dcdb62 --- /dev/null +++ b/0718-ehci-Don-t-access-packet-after-freeing-it.patch @@ -0,0 +1,40 @@ +From 9c8576aeca2d65e17748dc137f0e9abeb2959604 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 14 Nov 2012 16:21:36 +0000 +Subject: [PATCH 718/719] ehci: Don't access packet after freeing it + +ehci_state_writeback() will free the packet, so we should not access +the packet after calling ehci_state_writeback(). + +Signed-off-by: Hans de Goede +Signed-off-by: Gerd Hoffmann +(cherry picked from 30d68cf6e156b97fc462e18f38ce83f44702cd7f) +--- + hw/usb/hcd-ehci.c | 9 +++++---- + 1 file changed, 5 insertions(+), 4 deletions(-) + +diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c +index 46f6d99..4229061 100644 +--- a/hw/usb/hcd-ehci.c ++++ b/hw/usb/hcd-ehci.c +@@ -752,12 +752,13 @@ static EHCIPacket *ehci_alloc_packet(EHCIQueue *q) + static void ehci_free_packet(EHCIPacket *p) + { + if (p->async == EHCI_ASYNC_FINISHED) { +- int state = ehci_get_state(p->queue->ehci, p->queue->async); ++ EHCIQueue *q = p->queue; ++ int state = ehci_get_state(q->ehci, q->async); + /* This is a normal, but rare condition (cancel racing completion) */ + fprintf(stderr, "EHCI: Warning packet completed but not processed\n"); +- ehci_state_executing(p->queue); +- ehci_state_writeback(p->queue); +- ehci_set_state(p->queue->ehci, p->queue->async, state); ++ ehci_state_executing(q); ++ ehci_state_writeback(q); ++ ehci_set_state(q->ehci, q->async, state); + /* state_writeback recurses into us with async == EHCI_ASYNC_NONE!! */ + return; + } +-- +1.8.1.4 + diff --git a/0719-ehci-Fixup-q-qtdaddr-after-cancelling-an-already-com.patch b/0719-ehci-Fixup-q-qtdaddr-after-cancelling-an-already-com.patch new file mode 100644 index 0000000..2313ba3 --- /dev/null +++ b/0719-ehci-Fixup-q-qtdaddr-after-cancelling-an-already-com.patch @@ -0,0 +1,49 @@ +From f0a3e522543763cc5126283031309fdf6f8787c2 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Wed, 14 Nov 2012 16:21:37 +0000 +Subject: [PATCH 719/719] ehci: Fixup q->qtdaddr after cancelling an already + completed packet + +This avoids the q->qtdaddr == p->qtdaddr asserts we have triggering, when +a queue contains multiple completed packages when we cancel the queue. + +I triggered this with windows7 + async interrupt endpoint handling (*) ++ not detecting circles in ehci_fill_queue() properly, which makes the qtd +validation in ehci_fill_queue fail, causing cancellation of the queue on every +mouse event ... + +*) Which is not going upstream as it will cause loss of interrupt events on +migration. + +Signed-off-by: Hans de Goede +Signed-off-by: Gerd Hoffmann +(cherry picked from ff80ce599e0465cc6109a38bd3a8ca1890e88891) +--- + hw/usb/hcd-ehci.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c +index 4229061..6be11c5 100644 +--- a/hw/usb/hcd-ehci.c ++++ b/hw/usb/hcd-ehci.c +@@ -488,6 +488,7 @@ static const char *ehci_mmio_names[] = { + + static int ehci_state_executing(EHCIQueue *q); + static int ehci_state_writeback(EHCIQueue *q); ++static int ehci_state_advqueue(EHCIQueue *q); + + static const char *nr2str(const char **n, size_t len, uint32_t nr) + { +@@ -758,6 +759,9 @@ static void ehci_free_packet(EHCIPacket *p) + fprintf(stderr, "EHCI: Warning packet completed but not processed\n"); + ehci_state_executing(q); + ehci_state_writeback(q); ++ if (!(q->qh.token & QTD_TOKEN_HALT)) { ++ ehci_state_advqueue(q); ++ } + ehci_set_state(q->ehci, q->async, state); + /* state_writeback recurses into us with async == EHCI_ASYNC_NONE!! */ + return; +-- +1.8.1.4 + diff --git a/qemu.spec b/qemu.spec index f0897cb..aad959b 100644 --- a/qemu.spec +++ b/qemu.spec @@ -109,7 +109,7 @@ Summary: QEMU is a FAST! processor emulator Name: qemu Version: 1.2.2 -Release: 8%{?dist} +Release: 9%{?dist} # Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped Epoch: 2 License: GPLv2+ and LGPLv2+ and BSD @@ -545,6 +545,9 @@ Patch0715: 0715-pci-assign-Enable-MSIX-on-device-to-match-guest.patch # Fix QXL migration from F17 to F18 (bz #907916) Patch0716: 0716-qxl-change-rom-size-to-8192.patch Patch0717: 0717-qxl-Add-rom_size-compat-property-fix-migration-from-.patch +# Fix use after free + assert in ehci (bz #890320) +Patch0718: 0718-ehci-Don-t-access-packet-after-freeing-it.patch +Patch0719: 0719-ehci-Fixup-q-qtdaddr-after-cancelling-an-already-com.patch BuildRequires: SDL-devel @@ -1377,6 +1380,9 @@ CAC emulation development files. # Fix QXL migration from F17 to F18 (bz #907916) %patch0716 -p1 %patch0717 -p1 +# Fix use after free + assert in ehci (bz #890320) +%patch0718 -p1 +%patch0719 -p1 %build @@ -1986,6 +1992,9 @@ getent passwd qemu >/dev/null || \ %{_libdir}/pkgconfig/libcacard.pc %changelog +* Wed Apr 03 2013 Hans de Goede - 2:1.2.2-9 +- Fix use after free in ehci code (bz #890320) + * Mon Apr 01 2013 Cole Robinson - 2:1.2.2-8 - Don't use reserved word 'function' in systemtap files (bz #871286) - Fixes for iscsi dep