From f5177ea13d47aaaf98af341605bb83a4b432f2e2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 8 Sep 2014 09:23:14 +0200 Subject: [PATCH 10/15] vmstate_xhci_event: bug compat for rhel7.0.0 machine types (RHEV only) Message-id: <1410168194-26833-3-git-send-email-armbru@redhat.com> Patchwork-id: 60897 O-Subject: [PATCH v3 RHEV-7.1 qemu-kvm-rhev 2/2] vmstate_xhci_event: bug compat for rhel7.0.0 machine types (RHEV only) Bugzilla: 1136512 RH-Acked-by: Eduardo Habkost RH-Acked-by: Gerd Hoffmann RH-Acked-by: Dr. David Alan Gilbert (git) RH-Acked-by: Paolo Bonzini From: Laszlo Ersek Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1136512 The "vmstate_xhci_event.fields" member is a pointer to an array of VMStateField elements. The unnamed array (of static storage duration) comes from a compound literal. Commit 3afca1d6d fixed the undefined behavior (CVE-2014-5263) by adding a terminator element to this array, but between RHEL and RHEV we also need to look into the practical details of that undefined behavior. In debug builds (./configure --enable-debug), the compiler places the "vmstate_xhci_intr.fields" member's unnamed initializer array right after the "vmstate_xhci_event.fields" member's. This leads to infinite recursion (see the previous patch for details), but we don't ship debug builds for RHEL/RHEV. In a normal (optimized, official) build, the layout changes. The "vmstate_xhci_event.fields" member's unterminated initializer array is followed by the one of the "vmstate_xhci_slot.fields" member: (gdb) print (intptr_t)&vmstate_xhci_event.fields[7] - \ (intptr_t)&vmstate_xhci_slot.fields[0] $3 = 0 where "vmstate_xhci_slot.fields" is initialized from .fields = (VMStateField[]) { VMSTATE_BOOL(enabled, XHCISlot), VMSTATE_BOOL(addressed, XHCISlot), VMSTATE_END_OF_LIST() } The elements of this array are (only relevant members quoted): (gdb) print vmstate_xhci_slot.fields[0].offset $16 = 0 (gdb) print vmstate_xhci_slot.fields[0].size $17 = 1 (gdb) print vmstate_xhci_slot.fields[1].offset $18 = 1 (gdb) print vmstate_xhci_slot.fields[1].size $19 = 1 This means that the wire format for "vmstate_xhci_event" will include the byte at offset 0 and the byte at offset 1 from XHCIEvent, corresponding to part of the "XHCIEvent.type" member: (gdb) print vmstate_xhci_event.fields[0].name $23 = 0x5555558b12e7 "type" (gdb) print vmstate_xhci_event.fields[0].offset $24 = 0 (gdb) print vmstate_xhci_event.fields[0].size $25 = 4 In order to accommodate these bogus bytes, coming from an unpatched source side, we introduce two dummy XHCIEvent fields; otherwise the patched destination would reject the migration stream. For the reverse direction, we explicitly set the dummy bytes to the values that they used to take in an unpatched source, so that when the unpatched destination deserializes them into part of "XHCIEvent.type", said victim member still receives a correct value. The dummy fields have type uint8_t, not bool. The reason is that assignment to bool (in xhci_event_pre_save()) would entail conversion to bool, hence result in values 0 or 1. (See _Bool conversion rules and .) Migration of the dummy fields is gated with a compatibility flag. Downstream only because we control the compiler version and the build flags only in downstream. Suggested-by: Amit Shah Suggested-by: Dr. David Alan Gilbert Suggested-by: Markus Armbruster Suggested-by: Gerd Hoffmann Suggested-by: Paolo Bonzini Signed-off-by: Laszlo Ersek Signed-off-by: Markus Armbruster --- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/usb/hcd-xhci.c | 20 ++++++++++++++++++++ include/hw/usb.h | 3 +++ 4 files changed, 25 insertions(+) Signed-off-by: Miroslav Rezanina --- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + hw/usb/hcd-xhci.c | 20 ++++++++++++++++++++ include/hw/usb.h | 3 +++ 4 files changed, 25 insertions(+), 0 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 855c951..7f92f19 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -981,6 +981,7 @@ static void pc_compat_rhel700(MachineState *machine) legacy_acpi_table_size = 6418; /* see pc_compat_2_0() */ smbios_legacy_mode = true; has_reserved_memory = false; + migrate_cve_2014_5263_xhci_fields = true; } static void pc_init_rhel700(MachineState *machine) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 098151e..36e4e77 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -490,6 +490,7 @@ static void pc_q35_compat_rhel700(MachineState *machine) smbios_legacy_mode = true; has_reserved_memory = false; + migrate_cve_2014_5263_xhci_fields = true; } static void pc_q35_init_rhel700(MachineState *machine) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 58c4b11..051f574 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -418,6 +418,8 @@ typedef struct XHCIEvent { uint32_t flags; uint8_t slotid; uint8_t epid; + uint8_t cve_2014_5263_a; + uint8_t cve_2014_5263_b; } XHCIEvent; typedef struct XHCIInterrupter { @@ -3726,9 +3728,25 @@ static const VMStateDescription vmstate_xhci_slot = { } }; +static void xhci_event_pre_save(void *opaque) +{ + XHCIEvent *s = opaque; + + s->cve_2014_5263_a = ((uint8_t *)&s->type)[0]; + s->cve_2014_5263_b = ((uint8_t *)&s->type)[1]; +} + +bool migrate_cve_2014_5263_xhci_fields; + +static bool xhci_event_cve_2014_5263(void *opaque, int version_id) +{ + return migrate_cve_2014_5263_xhci_fields; +} + static const VMStateDescription vmstate_xhci_event = { .name = "xhci-event", .version_id = 1, + .pre_save = xhci_event_pre_save, .fields = (VMStateField[]) { VMSTATE_UINT32(type, XHCIEvent), VMSTATE_UINT32(ccode, XHCIEvent), @@ -3737,6 +3755,8 @@ static const VMStateDescription vmstate_xhci_event = { VMSTATE_UINT32(flags, XHCIEvent), VMSTATE_UINT8(slotid, XHCIEvent), VMSTATE_UINT8(epid, XHCIEvent), + VMSTATE_UINT8_TEST(cve_2014_5263_a, XHCIEvent, xhci_event_cve_2014_5263), + VMSTATE_UINT8_TEST(cve_2014_5263_b, XHCIEvent, xhci_event_cve_2014_5263), VMSTATE_END_OF_LIST() } }; diff --git a/include/hw/usb.h b/include/hw/usb.h index fad90bf..3222329 100644 --- a/include/hw/usb.h +++ b/include/hw/usb.h @@ -604,4 +604,7 @@ int usb_get_quirks(uint16_t vendor_id, uint16_t product_id, /* hcd-uhci.c -- RHEL-6 machine type compatibility */ extern bool ich9_uhci123_irqpin_override; +/* hcd-xhci.c -- rhel7.0.0 machine type compatibility */ +extern bool migrate_cve_2014_5263_xhci_fields; + #endif -- 1.7.1