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

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

[Laurent Vivier]

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

[PATCH v3 1/7] VirtioDeviceClass: Add an Error parameter to vmstate load member

[Laurent Vivier]

Replace error_report() by error_setg() when it's possible.

In scsi-bus, check if error has been set by load_request, and propagate
it to caller.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3888
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: new patch to propagate errors properly through the load_request chain
        includes changes made by Michael S. Tsirkin in
        https://lore.kernel.org/qemu-devel/3777347521a12252e228e4a6243c75250bf76626.1784898250.git.mst@redhat.com

 hw/block/virtio-blk.c       |  7 ++++---
 hw/char/virtio-serial-bus.c | 20 +++++++++++---------
 hw/scsi/esp.c               |  2 +-
 hw/scsi/mptsas.c            |  2 +-
 hw/scsi/scsi-bus.c          | 18 ++++++++++++++++--
 hw/scsi/scsi-disk.c         |  7 ++++---
 hw/scsi/scsi-generic.c      |  3 ++-
 hw/scsi/spapr_vscsi.c       |  7 ++++---
 hw/scsi/virtio-scsi.c       |  3 ++-
 hw/usb/dev-storage.c        |  2 +-
 hw/virtio/virtio.c          |  5 +++--
 include/hw/scsi/scsi.h      |  4 ++--
 include/hw/usb/msd.h        |  2 +-
 include/hw/virtio/virtio.h  |  2 +-
 14 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff4c..352b897c4ae5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1364,7 +1364,7 @@ static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
 }
 
 static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
-                                  int version_id)
+                                  int version_id, Error **errp)
 {
     VirtIOBlock *s = VIRTIO_BLK(vdev);
 
@@ -1377,8 +1377,9 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
             vq_idx = qemu_get_be32(f);
 
             if (vq_idx >= nvqs) {
-                error_report("Invalid virtqueue index in request list: %#x",
-                             vq_idx);
+                error_setg(errp,
+                           "Invalid virtqueue index in request list: 0x%x",
+                           vq_idx);
                 return -EINVAL;
             }
         }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c1973f0248fc..87bfe51b6e93 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -725,8 +725,8 @@ static void virtio_serial_post_load_timer_cb(void *opaque)
     s->post_load = NULL;
 }
 
-static int fetch_active_ports_list(QEMUFile *f,
-                                   VirtIOSerial *s, uint32_t nr_active_ports)
+static int fetch_active_ports_list(QEMUFile *f, VirtIOSerial *s,
+                                   uint32_t nr_active_ports, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
     uint32_t i;
@@ -749,6 +749,7 @@ static int fetch_active_ports_list(QEMUFile *f,
         id = qemu_get_be32(f);
         port = find_port_by_id(s, id);
         if (!port) {
+            error_setg(errp, "Invalid port id %u", id);
             return -EINVAL;
         }
 
@@ -776,7 +777,7 @@ static int fetch_active_ports_list(QEMUFile *f,
 }
 
 static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
-                                     int version_id)
+                                     int version_id, Error **errp)
 {
     VirtIOSerial *s = VIRTIO_SERIAL(vdev);
     uint32_t max_nr_ports, nr_active_ports, ports_map;
@@ -794,10 +795,8 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
         qemu_get_be32s(f, &ports_map);
 
         if (ports_map != s->ports_map[i]) {
-            /*
-             * Ports active on source and destination don't
-             * match. Fail migration.
-             */
+            error_setg(errp, "Ports active on source (%u) and destination (%u)"
+                       " don't match", ports_map, s->ports_map[i]);
             return -EINVAL;
         }
     }
@@ -805,8 +804,11 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
     qemu_get_be32s(f, &nr_active_ports);
 
     if (nr_active_ports) {
-        ret = fetch_active_ports_list(f, s, nr_active_ports);
-        if (ret) {
+        Error *local_err = NULL;
+
+        ret = fetch_active_ports_list(f, s, nr_active_ports, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
             return ret;
         }
     }
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 933271431ba0..a99bfe8e9428 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1539,7 +1539,7 @@ static uint64_t sysbus_esp_pdma_read(void *opaque, hwaddr addr,
     return val;
 }
 
-static void *esp_load_request(QEMUFile *f, SCSIRequest *req)
+static void *esp_load_request(QEMUFile *f, SCSIRequest *req, Error **errp)
 {
     ESPState *s = container_of(req->bus, ESPState, bus);
 
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 5df124c0ce50..45638af0afc3 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1230,7 +1230,7 @@ static void mptsas_save_request(QEMUFile *f, SCSIRequest *sreq)
     }
 }
 
-static void *mptsas_load_request(QEMUFile *f, SCSIRequest *sreq)
+static void *mptsas_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
 {
     SCSIBus *bus = sreq->bus;
     MPTSASState *s = container_of(bus, MPTSASState, bus);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index deb43d5560e3..9e9bb977bee3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1921,10 +1921,24 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
         req = scsi_req_new(s, tag, lun, buf, sizeof(buf), NULL);
         req->retry = (sbyte == 1);
         if (bus->info->load_request) {
-            req->hba_private = bus->info->load_request(f, req);
+            Error *local_err = NULL;
+
+            req->hba_private = bus->info->load_request(f, req, &local_err);
+            if (local_err) {
+                error_report_err(local_err);
+                scsi_req_unref(req);
+                return -1;
+            }
         }
         if (req->ops->load_request) {
-            req->ops->load_request(f, req);
+            Error *local_err = NULL;
+
+            req->ops->load_request(f, req, &local_err);
+            if (local_err) {
+                error_report_err(local_err);
+                scsi_req_unref(req);
+                return -1;
+            }
         }
 
         /* Just restart it later.  */
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 1b0cce128c5e..00f12439d1b6 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -181,7 +181,7 @@ static void scsi_disk_emulate_save_request(QEMUFile *f, SCSIRequest *req)
     }
 }
 
-static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req)
+static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req, Error **errp)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
@@ -204,12 +204,13 @@ static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req)
     qemu_iovec_init_external(&r->qiov, &r->iov, 1);
 }
 
-static void scsi_disk_emulate_load_request(QEMUFile *f, SCSIRequest *req)
+static void scsi_disk_emulate_load_request(QEMUFile *f, SCSIRequest *req,
+                                           Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
 
     if (s->migrate_emulated_scsi_request) {
-        scsi_disk_load_request(f, req);
+        scsi_disk_load_request(f, req, errp);
     }
 }
 
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 8999f3b72006..20deb9a21250 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -53,7 +53,8 @@ static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
     }
 }
 
-static void scsi_generic_load_request(QEMUFile *f, SCSIRequest *req)
+static void scsi_generic_load_request(QEMUFile *f, SCSIRequest *req,
+                                      Error **errp)
 {
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index b4c8f94d22ad..7768ec0bdd49 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -642,7 +642,7 @@ static void vscsi_save_request(QEMUFile *f, SCSIRequest *sreq)
                                    req->cur_desc_offset);
 }
 
-static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq)
+static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
 {
     SCSIBus *bus = sreq->bus;
     VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(bus->qbus.parent);
@@ -657,8 +657,9 @@ static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq)
     memset(req, 0, sizeof(*req));
     rc = vmstate_load_state(f, &vmstate_spapr_vscsi_req, req, 1, &local_err);
     if (rc) {
-        fprintf(stderr, "VSCSI: failed loading request tag#%u\n", sreq->tag);
-        error_report_err(local_err);
+        error_propagate_prepend(errp, local_err,
+                                "VSCSI: failed loading request tag#%u: ",
+                                sreq->tag);
         return NULL;
     }
     assert(req->active);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index bf64d1231a81..54667dc4f51d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -261,7 +261,8 @@ static void virtio_scsi_save_request(QEMUFile *f, SCSIRequest *sreq)
     qemu_put_virtqueue_element(vdev, f, &req->elem);
 }
 
-static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
+static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq,
+                                      Error **errp)
 {
     SCSIBus *bus = sreq->bus;
     VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 040cf1505181..d74aa087de66 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -556,7 +556,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
     }
 }
 
-void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
+void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req, Error **errp)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
 
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index daa5607338c9..64d780b1336e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3610,8 +3610,9 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
     virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
 
     if (vdc->load != NULL) {
-        ret = vdc->load(vdev, f, version_id);
-        if (ret) {
+        ret = vdc->load(vdev, f, version_id, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
             return ret;
         }
     }
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index c60c6e8810ea..51ccd02951c6 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -134,7 +134,7 @@ struct SCSIReqOps {
     uint8_t *(*get_buf)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
-    void (*load_request)(QEMUFile *f, SCSIRequest *req);
+    void (*load_request)(QEMUFile *f, SCSIRequest *req, Error **errp);
 };
 
 struct SCSIBusInfo {
@@ -150,7 +150,7 @@ struct SCSIBusInfo {
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
-    void *(*load_request)(QEMUFile *f, SCSIRequest *req);
+    void *(*load_request)(QEMUFile *f, SCSIRequest *req, Error **errp);
     void (*free_request)(SCSIBus *bus, void *priv);
 
     /*
diff --git a/include/hw/usb/msd.h b/include/hw/usb/msd.h
index 125d2c218f6d..167e7d3f2fe0 100644
--- a/include/hw/usb/msd.h
+++ b/include/hw/usb/msd.h
@@ -51,5 +51,5 @@ DECLARE_INSTANCE_CHECKER(MSDState, USB_STORAGE_DEV,
 void usb_msd_transfer_data(SCSIRequest *req, uint32_t len);
 void usb_msd_command_complete(SCSIRequest *req, size_t resid);
 void usb_msd_request_cancelled(SCSIRequest *req);
-void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req);
+void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req, Error **errp);
 void usb_msd_handle_reset(USBDevice *dev);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c99cb19d8865..bcb03154e243 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -268,7 +268,7 @@ struct VirtioDeviceClass {
      * use vmsd for new devices.
      */
     void (*save)(VirtIODevice *vdev, QEMUFile *f);
-    int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id);
+    int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id, Error **errp);
     /* Post load hook in vmsd is called early while device is processed, and
      * when VirtIODevice isn't fully initialized.  Devices should use this instead,
      * unless they specifically want to verify the migration stream as it's
-- 
2.54.0

[PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

[Laurent Vivier]

The migration restore path reads nr_active_ports from the incoming
stream and passes it directly to fetch_active_ports_list(), which
uses it to size a heap allocation. A crafted migration stream can set
this field to a very large value, causing QEMU to attempt a
multi-gigabyte allocation and abort.

Fix this by checking nr_active_ports against the configured
max_virtserial_ports before calling fetch_active_ports_list().

Cc: qemu-stable@nongnu.org
Fixes: 6663a1956eb6 ("virtio-serial-bus: Maintain guest and host port open/close state")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---

Notes:
    v2: add error_setg() call with descriptive error message

 hw/char/virtio-serial-bus.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 87bfe51b6e93..4a5507e2f755 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -803,6 +803,12 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
 
     qemu_get_be32s(f, &nr_active_ports);
 
+    if (nr_active_ports > max_nr_ports) {
+        error_setg(errp, "Invalid number of active ports %u > %u", nr_active_ports,
+                   max_nr_ports);
+        return -EINVAL;
+    }
+
     if (nr_active_ports) {
         Error *local_err = NULL;
 
-- 
2.54.0

[PATCH v3 3/7] virtio: do not crash QEMU on migration errors

[Laurent Vivier]

From: "Michael S. Tsirkin" <mst@redhat.com>

Currently virtio-blk, virtio-serial and virtio-scsi all crash on invalid
migration streams because qemu_get_virtqueue_element has no way to
detect and report mapping failures.

Not nice.

Let's propagate mapping errors through qemu_get_virtqueue_element and
fail migration instead.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3888
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
[lvivier: use errp rather than qemu_file_set_error() to report error]
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: new patch from Michael S. Tsirkin
        https://lore.kernel.org/qemu-devel/a0afc9ea82d68f3447c9f58d89c9b46fa074d6c3.1784898250.git.mst@redhat.com/
        Modified to use errp parameter with error_setg() instead of
        qemu_file_set_error() to report errors in virtio-blk,
        virtio-serial, and virtio-scsi

 hw/block/virtio-blk.c       |  4 ++++
 hw/char/virtio-serial-bus.c |  4 ++++
 hw/scsi/virtio-scsi.c       |  4 ++++
 hw/virtio/virtio.c          | 48 +++++++++++++++++++++++++++----------
 include/hw/virtio/virtio.h  |  2 +-
 5 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 352b897c4ae5..ccd6ad404ca0 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1385,6 +1385,10 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
         }
 
         req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
+        if (!req) {
+            error_setg(errp, "Failed to restore virtio-blk request");
+            return -EINVAL;
+        }
         virtio_blk_init_request(s, virtio_get_queue(vdev, vq_idx), req);
 
         WITH_QEMU_LOCK_GUARD(&s->rq_lock) {
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 4a5507e2f755..ecb7c5c7e507 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -764,6 +764,10 @@ static int fetch_active_ports_list(QEMUFile *f, VirtIOSerial *s,
 
             port->elem =
                 qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
+            if (!port->elem) {
+                error_setg(errp, "Failed to restore virtio-serial element");
+                return -EINVAL;
+            }
 
             /*
              *  Port was throttled on source machine.  Let's
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 54667dc4f51d..10eee3a4cff8 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -275,6 +275,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq,
     assert(n < vs->conf.num_queues);
     req = qemu_get_virtqueue_element(vdev, f,
                                      sizeof(VirtIOSCSIReq) + vs->cdb_size);
+    if (!req) {
+        error_setg(errp, "Failed to restore virtio-scsi request");
+        return NULL;
+    }
     virtio_scsi_init_req(s, vs->cmd_vqs[n], req);
 
     if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 64d780b1336e..02f75a5e281b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1680,36 +1680,55 @@ static void virtqueue_undo_map_desc(AddressSpace *as,
     }
 }
 
-static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
+static bool virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
                                 hwaddr *addr, unsigned int num_sg,
                                 bool is_write)
 {
     unsigned int i;
     hwaddr len;
+    DMADirection dir = is_write ? DMA_DIRECTION_FROM_DEVICE :
+                                 DMA_DIRECTION_TO_DEVICE;
 
     for (i = 0; i < num_sg; i++) {
         len = sg[i].iov_len;
-        sg[i].iov_base = dma_memory_map(vdev->dma_as,
-                                        addr[i], &len, is_write ?
-                                        DMA_DIRECTION_FROM_DEVICE :
-                                        DMA_DIRECTION_TO_DEVICE,
-                                        MEMTXATTRS_UNSPECIFIED);
+        sg[i].iov_base = dma_memory_map(vdev->dma_as, addr[i], &len,
+                                        dir, MEMTXATTRS_UNSPECIFIED);
         if (!sg[i].iov_base) {
             error_report("virtio: error trying to map MMIO memory");
-            exit(1);
+            goto err_undo_map;
         }
         if (len != sg[i].iov_len) {
             error_report("virtio: unexpected memory split");
-            exit(1);
+            dma_memory_unmap(vdev->dma_as, sg[i].iov_base, len, dir, 0);
+            goto err_undo_map;
         }
     }
+    return true;
+
+err_undo_map:
+    while (i-- > 0) {
+        dma_memory_unmap(vdev->dma_as, sg[i].iov_base, sg[i].iov_len,
+                         dir, 0);
+    }
+    return false;
 }
 
-void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
+bool virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
 {
-    virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, elem->in_num, true);
-    virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, elem->out_num,
-                                                                        false);
+    if (!virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr,
+                             elem->in_num, true)) {
+        return false;
+    }
+    if (!virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr,
+                             elem->out_num, false)) {
+        for (unsigned int i = 0; i < elem->in_num; i++) {
+            dma_memory_unmap(vdev->dma_as, elem->in_sg[i].iov_base,
+                             elem->in_sg[i].iov_len,
+                             DMA_DIRECTION_FROM_DEVICE, 0);
+        }
+        return false;
+    }
+    return true;
 }
 
 static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num)
@@ -2206,7 +2225,10 @@ void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz)
         qemu_get_be32s(f, &elem->ndescs);
     }
 
-    virtqueue_map(vdev, elem);
+    if (!virtqueue_map(vdev, elem)) {
+        g_free(elem);
+        return NULL;
+    }
     return elem;
 }
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index bcb03154e243..d0c63500a58d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -320,7 +320,7 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
 void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len, unsigned int idx);
 
-void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
+bool virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
 void *virtqueue_pop(VirtQueue *vq, size_t sz);
 unsigned int virtqueue_drop_all(VirtQueue *vq);
 void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
-- 
2.54.0

[PATCH v3 4/7] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state

[Laurent Vivier]

qemu_get_virtqueue_element() uses assert() to check that the in_num
and out_num fields deserialized from the migration stream do not
exceed VIRTQUEUE_MAX_SIZE. A crafted migration stream can set these
fields to invalid values, hitting the assertion and aborting the
destination QEMU process.

Replace the assertions with a bounds check that returns NULL on
failure.

Cc: qemu-stable@nongnu.org
Fixes: 6bdc21c050a2 ("virtio: fix up max size checks")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3802
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: remove caller updates (now handled by patch 3 from Michael)

 hw/virtio/virtio.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 02f75a5e281b..c37ef01bf513 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2190,13 +2190,10 @@ void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz)
 
     qemu_get_buffer(f, (uint8_t *)&data, sizeof(VirtQueueElementOld));
 
-    /* TODO: teach all callers that this can fail, and return failure instead
-     * of asserting here.
-     * This is just one thing (there are probably more) that must be
-     * fixed before we can allow NDEBUG compilation.
-     */
-    assert(ARRAY_SIZE(data.in_addr) >= data.in_num);
-    assert(ARRAY_SIZE(data.out_addr) >= data.out_num);
+    if (data.in_num > ARRAY_SIZE(data.in_addr) ||
+        data.out_num > ARRAY_SIZE(data.out_addr)) {
+        return NULL;
+    }
 
     elem = virtqueue_alloc_element(sz, data.out_num, data.in_num);
     elem->index = data.index;
-- 
2.54.0

[PATCH v3 5/7] mptsas: do not crash QEMU on migration errors

[Laurent Vivier]

From: "Michael S. Tsirkin" <mst@redhat.com>

Currently mptsas asserts on invalid sg list count in the migration
stream.

Fail migration gracefully instead.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
[lvivier: use errp rather than qemu_file_set_error() to report error]
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: new patch from Michael S. Tsirkin
        https://lore.kernel.org/qemu-devel/258d84f5af807e0103bf6f59165dd7450918e58c.1784898250.git.mst@redhat.com/
        Modified to use errp parameter with error_setg() instead of
        qemu_file_set_error() to report error

 hw/scsi/mptsas.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 45638af0afc3..34ceb6b5d25e 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -36,6 +36,8 @@
 #include "qapi/error.h"
 #include "mptsas.h"
 #include "migration/qemu-file-types.h"
+#include "migration/qemu-file.h"
+#include "qemu/error-report.h"
 #include "migration/vmstate.h"
 #include "mpi.h"
 
@@ -1242,12 +1244,12 @@ static void *mptsas_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
     qemu_get_buffer(f, (unsigned char *)&req->scsi_io, sizeof(req->scsi_io));
 
     n = qemu_get_be32(f);
-    /* TODO: add a way for SCSIBusInfo's load_request to fail,
-     * and fail migration instead of asserting here.
-     * This is just one thing (there are probably more) that must be
-     * fixed before we can allow NDEBUG compilation.
-     */
-    assert(n >= 0);
+    if (n < 0) {
+        error_setg(errp, "mptsas: invalid sg list count %d in migration stream",
+                   n);
+        g_free(req);
+        return NULL;
+    }
 
     pci_dma_sglist_init(&req->qsg, pci, n);
     for (i = 0; i < n; i++) {
-- 
2.54.0

[PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream

[Laurent Vivier]

virtio_scsi_load_request() uses assert() and exit(1) for conditions
that can be triggered by a crafted migration stream: an out-of-range
queue index, a malformed SCSI request, or a command mode mismatch.

Replace these with proper error returns so the migration fails
gracefully instead of aborting the destination QEMU process.

Cc: qemu-stable@nongnu.org
Fixes: 5db1764cc1f6 ("virtio-scsi: add migration support")
Fixes: d2ad7dd46e72 ("virtio-scsi: add multiqueue capability")
Fixes: 36b15c79aa1b ("virtio-scsi: start preparing for any_layout")
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: add error_setg() calls with descriptive error messages for
        all three error paths

 hw/scsi/virtio-scsi.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 10eee3a4cff8..8814b246048f 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -272,7 +272,12 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq,
     uint32_t n;
 
     qemu_get_be32s(f, &n);
-    assert(n < vs->conf.num_queues);
+    if (n >= vs->conf.num_queues) {
+        error_setg(errp, "Invalid queues number %u > %u", n,
+                   vs->conf.num_queues);
+        return NULL;
+    }
+
     req = qemu_get_virtqueue_element(vdev, f,
                                      sizeof(VirtIOSCSIReq) + vs->cdb_size);
     if (!req) {
@@ -283,14 +288,20 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq,
 
     if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
                               sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) {
-        error_report("invalid SCSI request migration data");
-        exit(1);
+        error_setg(errp, "invalid SCSI request migration data");
+        virtio_scsi_free_req(req);
+        return NULL;
     }
 
     scsi_req_ref(sreq);
     req->sreq = sreq;
-    if (req->sreq->cmd.mode != SCSI_XFER_NONE) {
-        assert(req->sreq->cmd.mode == req->mode);
+    if (req->sreq->cmd.mode != SCSI_XFER_NONE &&
+        req->sreq->cmd.mode != req->mode) {
+        error_setg(errp, "Invalid SCSI request mode %u",
+                   req->sreq->cmd.mode);
+        scsi_req_unref(sreq);
+        virtio_scsi_free_req(req);
+        return NULL;
     }
     return req;
 }
-- 
2.54.0

[PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors

[Laurent Vivier]

Currently vscsi asserts on invalid SCSI requests.

Fail migration gracefully instead.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    v2: New patch to manage vscsi

 hw/scsi/spapr_vscsi.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 7768ec0bdd49..b6a0411e9c44 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -650,9 +650,17 @@ static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
     int rc;
     Error *local_err = NULL;
 
-    assert(sreq->tag < VSCSI_REQ_LIMIT);
+    if (sreq->tag >= VSCSI_REQ_LIMIT) {
+        error_setg(errp, "VSCSI: request tag#%u out of range (max %d)",
+                   sreq->tag, VSCSI_REQ_LIMIT);
+        return NULL;
+    }
+
     req = &s->reqs[sreq->tag];
-    assert(!req->active);
+    if (req->active) {
+        error_setg(errp, "VSCSI: request tag#%u already active", sreq->tag);
+        return NULL;
+    }
 
     memset(req, 0, sizeof(*req));
     rc = vmstate_load_state(f, &vmstate_spapr_vscsi_req, req, 1, &local_err);
@@ -662,7 +670,11 @@ static void *vscsi_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
                                 sreq->tag);
         return NULL;
     }
-    assert(req->active);
+    if (!req->active) {
+        error_setg(errp, "VSCSI: request tag#%u not active after load",
+                   sreq->tag);
+        return NULL;
+    }
 
     req->sreq = scsi_req_ref(sreq);
 
-- 
2.54.0

Re: [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

[Thomas Huth]

On 30/07/2026 01.18, Laurent Vivier wrote:
> The migration restore path reads nr_active_ports from the incoming
> stream and passes it directly to fetch_active_ports_list(), which
> uses it to size a heap allocation. A crafted migration stream can set
> this field to a very large value, causing QEMU to attempt a
> multi-gigabyte allocation and abort.
> 
> Fix this by checking nr_active_ports against the configured
> max_virtserial_ports before calling fetch_active_ports_list().
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 6663a1956eb6 ("virtio-serial-bus: Maintain guest and host port open/close state")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Notes:
>      v2: add error_setg() call with descriptive error message
> 
>   hw/char/virtio-serial-bus.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 87bfe51b6e93..4a5507e2f755 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -803,6 +803,12 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
>   
>       qemu_get_be32s(f, &nr_active_ports);
>   
> +    if (nr_active_ports > max_nr_ports) {
> +        error_setg(errp, "Invalid number of active ports %u > %u", nr_active_ports,
> +                   max_nr_ports);
> +        return -EINVAL;
> +    }
> +
>       if (nr_active_ports) {
>           Error *local_err = NULL;
>   

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Michael S. Tsirkin]

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

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

[Michael S. Tsirkin]

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 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

On Thu, Jul 30, 2026 at 01:19:03AM +0200, Laurent Vivier wrote:
> virtio_scsi_load_request() uses assert() and exit(1) for conditions
> that can be triggered by a crafted migration stream: an out-of-range
> queue index, a malformed SCSI request, or a command mode mismatch.
> 
> Replace these with proper error returns so the migration fails
> gracefully instead of aborting the destination QEMU process.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 5db1764cc1f6 ("virtio-scsi: add migration support")
> Fixes: d2ad7dd46e72 ("virtio-scsi: add multiqueue capability")
> Fixes: 36b15c79aa1b ("virtio-scsi: start preparing for any_layout")
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> 
> Notes:
>     v2: add error_setg() calls with descriptive error messages for
>         all three error paths
> 
>  hw/scsi/virtio-scsi.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Thu, Jul 30, 2026 at 01:19:04AM +0200, Laurent Vivier wrote:
> Currently vscsi asserts on invalid SCSI requests.
> 
> Fail migration gracefully instead.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> 
> Notes:
>     v2: New patch to manage vscsi
> 
>  hw/scsi/spapr_vscsi.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors

[Laurent Vivier]

On 7/30/26 01:19, Laurent Vivier wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> 
> Currently mptsas asserts on invalid sg list count in the migration
> stream.
> 
> Fail migration gracefully instead.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> [lvivier: use errp rather than qemu_file_set_error() to report error]
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> 
> Notes:
>      v2: new patch from Michael S. Tsirkin
>          https://lore.kernel.org/qemu-devel/258d84f5af807e0103bf6f59165dd7450918e58c.1784898250.git.mst@redhat.com/
>          Modified to use errp parameter with error_setg() instead of
>          qemu_file_set_error() to report error
> 
>   hw/scsi/mptsas.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
> index 45638af0afc3..34ceb6b5d25e 100644
> --- a/hw/scsi/mptsas.c
> +++ b/hw/scsi/mptsas.c
> @@ -36,6 +36,8 @@
>   #include "qapi/error.h"
>   #include "mptsas.h"
>   #include "migration/qemu-file-types.h"
> +#include "migration/qemu-file.h"

When I removed qemu_file_set_error() I forgot to remove this include.
Perhaps it can be removed on merge? Otherwise I will send a v3.

Thanks,
Laurent

> +#include "qemu/error-report.h"
>   #include "migration/vmstate.h"
>   #include "mpi.h"
>   
> @@ -1242,12 +1244,12 @@ static void *mptsas_load_request(QEMUFile *f, SCSIRequest *sreq, Error **errp)
>       qemu_get_buffer(f, (unsigned char *)&req->scsi_io, sizeof(req->scsi_io));
>   
>       n = qemu_get_be32(f);
> -    /* TODO: add a way for SCSIBusInfo's load_request to fail,
> -     * and fail migration instead of asserting here.
> -     * This is just one thing (there are probably more) that must be
> -     * fixed before we can allow NDEBUG compilation.
> -     */
> -    assert(n >= 0);
> +    if (n < 0) {
> +        error_setg(errp, "mptsas: invalid sg list count %d in migration stream",
> +                   n);
> +        g_free(req);
> +        return NULL;
> +    }
>   
>       pci_dma_sglist_init(&req->qsg, pci, n);
>       for (i = 0; i < n; i++) {