[PATCH v3 0/7] Harden virtio migration load paths against crafted streams

Laurent Vivier posted 7 patches 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260729231904.775331-1-lvivier@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Amit Shah <amit@kernel.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>
hw/block/virtio-blk.c       | 11 +++++--
hw/char/virtio-serial-bus.c | 30 +++++++++++------
hw/scsi/esp.c               |  2 +-
hw/scsi/mptsas.c            | 16 ++++++----
hw/scsi/scsi-bus.c          | 18 +++++++++--
hw/scsi/scsi-disk.c         |  7 ++--
hw/scsi/scsi-generic.c      |  3 +-
hw/scsi/spapr_vscsi.c       | 25 +++++++++++----
hw/scsi/virtio-scsi.c       | 28 ++++++++++++----
hw/usb/dev-storage.c        |  2 +-
hw/virtio/virtio.c          | 64 ++++++++++++++++++++++++-------------
include/hw/scsi/scsi.h      |  4 +--
include/hw/usb/msd.h        |  2 +-
include/hw/virtio/virtio.h  |  4 +--
14 files changed, 150 insertions(+), 66 deletions(-)
[PATCH v3 0/7] Harden virtio migration load paths against crafted streams
Posted by Laurent Vivier 2 months ago
A crafted migration stream can crash the destination QEMU process
through unvalidated fields in the virtio device state: an unbounded
allocation in virtio-serial, reachable assertions in the shared
virtqueue element deserializer, assert()/exit(1) calls in
virtio-scsi and spapr-vscsi request loading, and missing error
propagation through the SCSI and virtio device load paths.

These are hardening fixes: the destination QEMU is in a paused
pre-start state and the source VM is unaffected by a failed migration.

Patch 1 adds an Error** parameter to the virtio and SCSI load_request
callbacks, allowing proper error propagation instead of error_report()
or silent failures. This provides the infrastructure for the remaining
patches.

Patch 2 validates the virtio-serial nr_active_ports count against the
configured maximum before allocating the post-load array.

Patches 3 and 5 are from Michael S. Tsirkin, modified to use the new
Error** parameter:

Patch 3 makes virtqueue_map() return bool instead of calling exit(1)
on mapping failures, and propagates errors through
qemu_get_virtqueue_element() to virtio-blk, virtio-serial, and
virtio-scsi.

Patch 4 replaces the assertions in qemu_get_virtqueue_element() with
a bounds check returning NULL on invalid in_num/out_num counts.

Patch 5 replaces the assertion in mptsas_load_request() with proper
error handling.

Patch 6 replaces the remaining assert() and exit(1) calls in
virtio_scsi_load_request() with proper error returns.

Patch 7 replaces the assert() calls in vscsi_load_request() with
proper error returns and propagation.

v2:
- New patch 1 to add Error** parameter to load_request callbacks
- Patches 2, 4, 6, 7: add error_setg() calls with descriptive messages
- New patches 3 and 5 from Michael S. Tsirkin (modified to use errp)
- Patch 4: remove caller updates (now handled by patch 3)
- New patch 7: harden spapr_vscsi load_request

Tested with migration round-trips for virtio-serial (0 to 511 ports),
virtio-blk (1-2 disks), virtio-scsi (1-2 disks), mptsas1068
(with and without scsi-hd), and spapr-vscsi, plus the original PoC
reproducers for issues #3801, #3802, and #3888. ppc64 qtests pass.

Laurent Vivier (5):
  VirtioDeviceClass: Add an Error parameter to vmstate load member
  hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
  hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state
  hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
  hw/scsi/spapr_vscsi: do not crash QEMU on migration errors

Michael S. Tsirkin (2):
  virtio: do not crash QEMU on migration errors
  mptsas: do not crash QEMU on migration errors

 hw/block/virtio-blk.c       | 16 ++++++--
 hw/char/virtio-serial-bus.c | 33 ++++++++++----
 hw/scsi/esp.c               |  2 +-
 hw/scsi/mptsas.c            | 18 +++++---
 hw/scsi/scsi-bus.c          | 25 +++++++++--
 hw/scsi/scsi-disk.c         | 10 +++--
 hw/scsi/scsi-generic.c      |  3 +-
 hw/scsi/spapr_vscsi.c       | 18 +++++++--
 hw/scsi/virtio-scsi.c       | 27 ++++++++++--
 hw/usb/dev-storage.c        |  2 +-
 hw/virtio/virtio.c          | 82 ++++++++++++++++++++++++++-----------
 include/hw/scsi/scsi.h      |  4 +-
 include/hw/usb/msd.h        |  2 +-
 include/hw/virtio/virtio.h  |  4 +-
 14 files changed, 182 insertions(+), 62 deletions(-)

Laurent Vivier (5):
  VirtioDeviceClass: Add an Error parameter to vmstate load member
  hw/char/virtio-serial-bus: validate nr_active_ports from migration
    stream
  hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid
    state
  hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid
    stream
  hw/scsi/spapr_vscsi: do not crash QEMU on migration errors

Michael S. Tsirkin (2):
  virtio: do not crash QEMU on migration errors
  mptsas: do not crash QEMU on migration errors

 hw/block/virtio-blk.c       | 11 +++++--
 hw/char/virtio-serial-bus.c | 30 +++++++++++------
 hw/scsi/esp.c               |  2 +-
 hw/scsi/mptsas.c            | 16 ++++++----
 hw/scsi/scsi-bus.c          | 18 +++++++++--
 hw/scsi/scsi-disk.c         |  7 ++--
 hw/scsi/scsi-generic.c      |  3 +-
 hw/scsi/spapr_vscsi.c       | 25 +++++++++++----
 hw/scsi/virtio-scsi.c       | 28 ++++++++++++----
 hw/usb/dev-storage.c        |  2 +-
 hw/virtio/virtio.c          | 64 ++++++++++++++++++++++++-------------
 include/hw/scsi/scsi.h      |  4 +--
 include/hw/usb/msd.h        |  2 +-
 include/hw/virtio/virtio.h  |  4 +--
 14 files changed, 150 insertions(+), 66 deletions(-)

-- 
2.54.0
Re: [PATCH v3 0/7] Harden virtio migration load paths against crafted streams
Posted by Michael S. Tsirkin 1 month, 4 weeks ago
On Thu, Jul 30, 2026 at 01:18:57AM +0200, Laurent Vivier wrote:
> A crafted migration stream can crash the destination QEMU process
> through unvalidated fields in the virtio device state: an unbounded
> allocation in virtio-serial, reachable assertions in the shared
> virtqueue element deserializer, assert()/exit(1) calls in
> virtio-scsi and spapr-vscsi request loading, and missing error
> propagation through the SCSI and virtio device load paths.
> 
> These are hardening fixes: the destination QEMU is in a paused
> pre-start state and the source VM is unaffected by a failed migration.
> 
> Patch 1 adds an Error** parameter to the virtio and SCSI load_request
> callbacks, allowing proper error propagation instead of error_report()
> or silent failures. This provides the infrastructure for the remaining
> patches.
> 
> Patch 2 validates the virtio-serial nr_active_ports count against the
> configured maximum before allocating the post-load array.
> 
> Patches 3 and 5 are from Michael S. Tsirkin, modified to use the new
> Error** parameter:
> 
> Patch 3 makes virtqueue_map() return bool instead of calling exit(1)
> on mapping failures, and propagates errors through
> qemu_get_virtqueue_element() to virtio-blk, virtio-serial, and
> virtio-scsi.
> 
> Patch 4 replaces the assertions in qemu_get_virtqueue_element() with
> a bounds check returning NULL on invalid in_num/out_num counts.
> 
> Patch 5 replaces the assertion in mptsas_load_request() with proper
> error handling.
> 
> Patch 6 replaces the remaining assert() and exit(1) calls in
> virtio_scsi_load_request() with proper error returns.
> 
> Patch 7 replaces the assert() calls in vscsi_load_request() with
> proper error returns and propagation.


who's merging this btw? me?

> v2:
> - New patch 1 to add Error** parameter to load_request callbacks
> - Patches 2, 4, 6, 7: add error_setg() calls with descriptive messages
> - New patches 3 and 5 from Michael S. Tsirkin (modified to use errp)
> - Patch 4: remove caller updates (now handled by patch 3)
> - New patch 7: harden spapr_vscsi load_request
> 
> Tested with migration round-trips for virtio-serial (0 to 511 ports),
> virtio-blk (1-2 disks), virtio-scsi (1-2 disks), mptsas1068
> (with and without scsi-hd), and spapr-vscsi, plus the original PoC
> reproducers for issues #3801, #3802, and #3888. ppc64 qtests pass.
> 
> Laurent Vivier (5):
>   VirtioDeviceClass: Add an Error parameter to vmstate load member
>   hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
>   hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state
>   hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
>   hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
> 
> Michael S. Tsirkin (2):
>   virtio: do not crash QEMU on migration errors
>   mptsas: do not crash QEMU on migration errors
> 
>  hw/block/virtio-blk.c       | 16 ++++++--
>  hw/char/virtio-serial-bus.c | 33 ++++++++++----
>  hw/scsi/esp.c               |  2 +-
>  hw/scsi/mptsas.c            | 18 +++++---
>  hw/scsi/scsi-bus.c          | 25 +++++++++--
>  hw/scsi/scsi-disk.c         | 10 +++--
>  hw/scsi/scsi-generic.c      |  3 +-
>  hw/scsi/spapr_vscsi.c       | 18 +++++++--
>  hw/scsi/virtio-scsi.c       | 27 ++++++++++--
>  hw/usb/dev-storage.c        |  2 +-
>  hw/virtio/virtio.c          | 82 ++++++++++++++++++++++++++-----------
>  include/hw/scsi/scsi.h      |  4 +-
>  include/hw/usb/msd.h        |  2 +-
>  include/hw/virtio/virtio.h  |  4 +-
>  14 files changed, 182 insertions(+), 62 deletions(-)
> 
> Laurent Vivier (5):
>   VirtioDeviceClass: Add an Error parameter to vmstate load member
>   hw/char/virtio-serial-bus: validate nr_active_ports from migration
>     stream
>   hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid
>     state
>   hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid
>     stream
>   hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
> 
> Michael S. Tsirkin (2):
>   virtio: do not crash QEMU on migration errors
>   mptsas: do not crash QEMU on migration errors
> 
>  hw/block/virtio-blk.c       | 11 +++++--
>  hw/char/virtio-serial-bus.c | 30 +++++++++++------
>  hw/scsi/esp.c               |  2 +-
>  hw/scsi/mptsas.c            | 16 ++++++----
>  hw/scsi/scsi-bus.c          | 18 +++++++++--
>  hw/scsi/scsi-disk.c         |  7 ++--
>  hw/scsi/scsi-generic.c      |  3 +-
>  hw/scsi/spapr_vscsi.c       | 25 +++++++++++----
>  hw/scsi/virtio-scsi.c       | 28 ++++++++++++----
>  hw/usb/dev-storage.c        |  2 +-
>  hw/virtio/virtio.c          | 64 ++++++++++++++++++++++++-------------
>  include/hw/scsi/scsi.h      |  4 +--
>  include/hw/usb/msd.h        |  2 +-
>  include/hw/virtio/virtio.h  |  4 +--
>  14 files changed, 150 insertions(+), 66 deletions(-)
> 
> -- 
> 2.54.0
Re: [PATCH v3 0/7] Harden virtio migration load paths against crafted streams
Posted by Michael S. Tsirkin 1 month, 4 weeks ago
On Thu, Jul 30, 2026 at 01:18:57AM +0200, Laurent Vivier wrote:
> A crafted migration stream can crash the destination QEMU process
> through unvalidated fields in the virtio device state: an unbounded
> allocation in virtio-serial, reachable assertions in the shared
> virtqueue element deserializer, assert()/exit(1) calls in
> virtio-scsi and spapr-vscsi request loading, and missing error
> propagation through the SCSI and virtio device load paths.
> 
> These are hardening fixes: the destination QEMU is in a paused
> pre-start state and the source VM is unaffected by a failed migration.
> 
> Patch 1 adds an Error** parameter to the virtio and SCSI load_request
> callbacks, allowing proper error propagation instead of error_report()
> or silent failures. This provides the infrastructure for the remaining
> patches.
> 
> Patch 2 validates the virtio-serial nr_active_ports count against the
> configured maximum before allocating the post-load array.
> 
> Patches 3 and 5 are from Michael S. Tsirkin, modified to use the new
> Error** parameter:
> 
> Patch 3 makes virtqueue_map() return bool instead of calling exit(1)
> on mapping failures, and propagates errors through
> qemu_get_virtqueue_element() to virtio-blk, virtio-serial, and
> virtio-scsi.
> 
> Patch 4 replaces the assertions in qemu_get_virtqueue_element() with
> a bounds check returning NULL on invalid in_num/out_num counts.
> 
> Patch 5 replaces the assertion in mptsas_load_request() with proper
> error handling.
> 
> Patch 6 replaces the remaining assert() and exit(1) calls in
> virtio_scsi_load_request() with proper error returns.
> 
> Patch 7 replaces the assert() calls in vscsi_load_request() with
> proper error returns and propagation.


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> v2:
> - New patch 1 to add Error** parameter to load_request callbacks
> - Patches 2, 4, 6, 7: add error_setg() calls with descriptive messages
> - New patches 3 and 5 from Michael S. Tsirkin (modified to use errp)
> - Patch 4: remove caller updates (now handled by patch 3)
> - New patch 7: harden spapr_vscsi load_request
> 
> Tested with migration round-trips for virtio-serial (0 to 511 ports),
> virtio-blk (1-2 disks), virtio-scsi (1-2 disks), mptsas1068
> (with and without scsi-hd), and spapr-vscsi, plus the original PoC
> reproducers for issues #3801, #3802, and #3888. ppc64 qtests pass.
> 
> Laurent Vivier (5):
>   VirtioDeviceClass: Add an Error parameter to vmstate load member
>   hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
>   hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state
>   hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
>   hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
> 
> Michael S. Tsirkin (2):
>   virtio: do not crash QEMU on migration errors
>   mptsas: do not crash QEMU on migration errors
> 
>  hw/block/virtio-blk.c       | 16 ++++++--
>  hw/char/virtio-serial-bus.c | 33 ++++++++++----
>  hw/scsi/esp.c               |  2 +-
>  hw/scsi/mptsas.c            | 18 +++++---
>  hw/scsi/scsi-bus.c          | 25 +++++++++--
>  hw/scsi/scsi-disk.c         | 10 +++--
>  hw/scsi/scsi-generic.c      |  3 +-
>  hw/scsi/spapr_vscsi.c       | 18 +++++++--
>  hw/scsi/virtio-scsi.c       | 27 ++++++++++--
>  hw/usb/dev-storage.c        |  2 +-
>  hw/virtio/virtio.c          | 82 ++++++++++++++++++++++++++-----------
>  include/hw/scsi/scsi.h      |  4 +-
>  include/hw/usb/msd.h        |  2 +-
>  include/hw/virtio/virtio.h  |  4 +-
>  14 files changed, 182 insertions(+), 62 deletions(-)
> 
> Laurent Vivier (5):
>   VirtioDeviceClass: Add an Error parameter to vmstate load member
>   hw/char/virtio-serial-bus: validate nr_active_ports from migration
>     stream
>   hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid
>     state
>   hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid
>     stream
>   hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
> 
> Michael S. Tsirkin (2):
>   virtio: do not crash QEMU on migration errors
>   mptsas: do not crash QEMU on migration errors
> 
>  hw/block/virtio-blk.c       | 11 +++++--
>  hw/char/virtio-serial-bus.c | 30 +++++++++++------
>  hw/scsi/esp.c               |  2 +-
>  hw/scsi/mptsas.c            | 16 ++++++----
>  hw/scsi/scsi-bus.c          | 18 +++++++++--
>  hw/scsi/scsi-disk.c         |  7 ++--
>  hw/scsi/scsi-generic.c      |  3 +-
>  hw/scsi/spapr_vscsi.c       | 25 +++++++++++----
>  hw/scsi/virtio-scsi.c       | 28 ++++++++++++----
>  hw/usb/dev-storage.c        |  2 +-
>  hw/virtio/virtio.c          | 64 ++++++++++++++++++++++++-------------
>  include/hw/scsi/scsi.h      |  4 +--
>  include/hw/usb/msd.h        |  2 +-
>  include/hw/virtio/virtio.h  |  4 +--
>  14 files changed, 150 insertions(+), 66 deletions(-)
> 
> -- 
> 2.54.0