From a0a7c88ceec2443192ec27242b176b66f82dd74a Mon Sep 17 00:00:00 2001

From: Laszlo Ersek <lersek@redhat.com>

Date: Mon, 1 Sep 2014 13:36:53 +0200

Subject: [PATCH 2/6] vmstate_xhci_event: bug compat with RHEL-7.0 (RHEL only)

Message-id: <1409578613-11909-3-git-send-email-lersek@redhat.com>

Patchwork-id: 60782

O-Subject: [PATCH RHEL-7.0.z/RHEL-7.1.0 qemu-kvm 2/2] vmstate_xhci_event: bug compat with RHEL-7.0 (RHEL only)

Bugzilla: 1145055

RH-Acked-by: Dr. David Alan Gilbert (git) <dgilbert@redhat.com>

RH-Acked-by: Amit Shah <amit.shah@redhat.com>

RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

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. The previous patch fixed the undefined

behavior by adding a terminator element to this array, but in RHEL-7 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 in RHEL-7 we don't ship debug

builds.

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

<stdbool.h>.)

RHEL-only because we control the compiler version and the build flags only

in RHEL.

This is for CVE-2014-5263.

Suggested-by: Amit Shah <amit.shah@redhat.com>

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Suggested-by: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 hw/usb/hcd-xhci.c |   27 ++++++++++++++++++++-------

 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c

index dbdf6b1..9f97167 100644

--- a/hw/usb/hcd-xhci.c

+++ b/hw/usb/hcd-xhci.c

@@ -420,6 +420,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 {

@@ -3515,17 +3517,28 @@ 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];

+}

+

 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),

-        VMSTATE_UINT64(ptr,    XHCIEvent),

-        VMSTATE_UINT32(length, XHCIEvent),

-        VMSTATE_UINT32(flags,  XHCIEvent),

-        VMSTATE_UINT8(slotid,  XHCIEvent),

-        VMSTATE_UINT8(epid,    XHCIEvent),

+        VMSTATE_UINT32(type,           XHCIEvent),

+        VMSTATE_UINT32(ccode,          XHCIEvent),

+        VMSTATE_UINT64(ptr,            XHCIEvent),

+        VMSTATE_UINT32(length,         XHCIEvent),

+        VMSTATE_UINT32(flags,          XHCIEvent),

+        VMSTATE_UINT8(slotid,          XHCIEvent),

+        VMSTATE_UINT8(epid,            XHCIEvent),

+        VMSTATE_UINT8(cve_2014_5263_a, XHCIEvent),

+        VMSTATE_UINT8(cve_2014_5263_b, XHCIEvent),

         VMSTATE_END_OF_LIST()

     }

 };

-- 

1.7.1