[Qemu-devel] [PATCH v5 00/24] qemu state loading issues

[Michael S. Tsirkin]

Changes from v4:
    Addressed comments by Peter, David, Amit, Laszlo
    dropped vmxnet3 patches: will re-add when author
        addresses comments raised
    dropped stellaris patches: superceded by Peter's rewrite
    Added Peter's better fix for savevm crash

Changes from v3:
    Rewritten input validation in multiple patches using the new
    VMSTATE_VALIDATE macro.
    Addressed review comments from Peter Maydell,
    Andreas Färber, Don Koch and Dr. David Alan Gilbert.

The following is the list of patches unmodified from v4:
 vmstate: add VMS_MUST_EXIST
 vmstate: add VMSTATE_VALIDATE
 virtio-net: fix buffer overflow on invalid state load
    [patch unchanged, comment tweaked as suggested by Laszlo]
 virtio-net: out-of-bounds buffer write on invalid state load
 virtio: out-of-bounds buffer write on invalid state load
 ahci: fix buffer overrun on invalid state load
 hpet: fix buffer overrun on invalid state load
 hw/pci/pcie_aer.c: fix buffer overruns on invalid state load
 virtio: avoid buffer overrun on incoming migration
 pxa2xx: avoid buffer overrun on incoming migration
 zaurus: fix buffer overrun on invalid state load
 virtio-scsi: fix buffer overrun on invalid state load

The following is the list of patches unmodified from v1:
 virtio: out-of-bounds buffer write on invalid state load
 ahci: fix buffer overrun on invalid state load
 virtio: avoid buffer overrun on incoming migration


In some cases CVEs have been created to track specific issues.
Where available, CVE # is listed in the commit log.

I doubt it makes sense to push this urgently into 2.0.

Let's fix for 2.1, and backport as appropriate.

Cover latter from v1:
The state loading functionality was written under
the assumption that the state being loaded can be trusted. This is
mostly true, but we have identified at least two scenarios where it's
not:

* An attacker who has complete control over source qemu-kvm/node (via
  another flaw) and wants to attack destination node (source and
  destination for live migration). He can thus change the migration
  data that will be processed on the destination node, potentially
  allowing exploitation and remote code execution.

  Also, migration initiation is a privileged operation, but I think the
  attacker on the source node could probably fake some symptoms that
  would either make some automated process to start migrating off VMs
  from the node or make node admin to notice and start manual
  migration.

  MITM attack is not considered to be security relevant since the
  security between endpoints can be considered to be configuration
  issue.

* Saving/Loading state to/from file.

  For example, some bugzilla entries supply a savevm file
  and ask developer to load that to reproduce.
  
  After I have identified a first issue like this,
  a full audit of the qemu code base was done by Anthony Liguori, Michael
  Roth, myself and others, and found multiple instances where loading in
  invalid image would corrupt QEMU memory, in some instances making it
  possible to overwrite it with attacker-controlled data.
  
  This patchset is the result of that audit: it addresses this set of
  security issues by adding input validation and failing migration on
  invalid input.
  
  Considering the preconditions, I think that the impact on typical qemu usage 
is
  low.  Still, I think these patches make sense for qemu-stable.
  
  Lots of thanks to Stefan Hajnoczi, Gerd Hoffmann, Kevin Wolf, Paolo
  Bonzini and Hans de Goede, for help with the code audit.  Petr
  Matousek for review. I hope I didn't forget anyone involved, if I did
  I apologize in advance.
  
  I have parked them on my tree for now so they are not lost.
  
  Please review, and consider for master and stable.

Michael Roth (2):
  virtio: avoid buffer overrun on incoming migration
  openpic: avoid buffer overrun on incoming migration

Michael S. Tsirkin (21):
  vmstate: reduce code duplication
  vmstate: add VMS_MUST_EXIST
  vmstate: add VMSTATE_VALIDATE
  virtio-net: fix buffer overflow on invalid state load
  virtio-net: out-of-bounds buffer write on load
  virtio-net: out-of-bounds buffer write on invalid state load
  virtio: out-of-bounds buffer write on invalid state load
  ahci: fix buffer overrun on invalid state load
  hpet: fix buffer overrun on invalid state load
  hw/pci/pcie_aer.c: fix buffer overruns on invalid state load
  pl022: fix buffer overun on invalid state load
  vmstate: fix buffer overflow in target-arm/machine.c
  virtio: validate num_sg when mapping
  pxa2xx: avoid buffer overrun on incoming migration
  ssi-sd: fix buffer overrun on invalid state load
  ssd0323: fix buffer overun on invalid state load
  tsc210x: fix buffer overrun on invalid state load
  zaurus: fix buffer overrun on invalid state load
  virtio-scsi: fix buffer overrun on invalid state load
  vmstate: s/VMSTATE_INT32_LE/VMSTATE_INT32_POSITIVE_LE/
  usb: sanity check setup_index+setup_len in post_load

Peter Maydell (1):
  savevm: Ignore minimum_version_id_old if there is no load_state_old

 include/hw/virtio/virtio-net.h |   4 +-
 include/migration/vmstate.h    |  11 +++-
 hw/arm/pxa2xx.c                |   8 ++-
 hw/display/ssd0323.c           |  24 ++++++++
 hw/gpio/zaurus.c               |  10 ++++
 hw/ide/ahci.c                  |   2 +-
 hw/input/tsc210x.c             |  12 ++++
 hw/intc/openpic.c              |  14 ++++-
 hw/net/virtio-net.c            |  19 +++++--
 hw/pci/pci.c                   |   4 +-
 hw/pci/pcie_aer.c              |  10 +++-
 hw/scsi/virtio-scsi.c          |   9 +++
 hw/sd/ssi-sd.c                 |   8 +++
 hw/ssi/pl022.c                 |  14 +++++
 hw/timer/hpet.c                |  13 +++++
 hw/usb/bus.c                   |   4 +-
 hw/virtio/virtio.c             |  17 +++++-
 target-arm/machine.c           |   2 +-
 vmstate.c                      | 126 +++++++++++++++++++++++------------------
 docs/migration.txt             |  12 ++--
 20 files changed, 243 insertions(+), 80 deletions(-)

-- 
MST