* [PATCH v3 1/7] VirtioDeviceClass: Add an Error parameter to vmstate load member
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
@ 2026-07-29 23:18 ` Laurent Vivier
2026-07-29 23:18 ` [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:18 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
2026-07-29 23:18 ` [PATCH v3 1/7] VirtioDeviceClass: Add an Error parameter to vmstate load member Laurent Vivier
@ 2026-07-29 23:18 ` Laurent Vivier
2026-07-30 6:58 ` Thomas Huth
2026-07-29 23:19 ` [PATCH v3 3/7] virtio: do not crash QEMU on migration errors Laurent Vivier
` (6 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:18 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
Laurent Vivier, qemu-stable, Daniel P. Berrangé, Thomas Huth
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
2026-07-29 23:18 ` [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
@ 2026-07-30 6:58 ` Thomas Huth
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2026-07-30 6:58 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
qemu-stable, Daniel P. Berrangé
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/7] virtio: do not crash QEMU on migration errors
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
2026-07-29 23:18 ` [PATCH v3 1/7] VirtioDeviceClass: Add an Error parameter to vmstate load member Laurent Vivier
2026-07-29 23:18 ` [PATCH v3 2/7] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
@ 2026-07-29 23:19 ` Laurent Vivier
2026-07-29 23:19 ` [PATCH v3 4/7] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state Laurent Vivier
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:19 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 4/7] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (2 preceding siblings ...)
2026-07-29 23:19 ` [PATCH v3 3/7] virtio: do not crash QEMU on migration errors Laurent Vivier
@ 2026-07-29 23:19 ` Laurent Vivier
2026-07-29 23:19 ` [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors Laurent Vivier
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:19 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
Laurent Vivier, qemu-stable
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (3 preceding siblings ...)
2026-07-29 23:19 ` [PATCH v3 4/7] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state Laurent Vivier
@ 2026-07-29 23:19 ` Laurent Vivier
2026-07-30 14:01 ` Laurent Vivier
2026-07-29 23:19 ` [PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
` (3 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:19 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors
2026-07-29 23:19 ` [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors Laurent Vivier
@ 2026-07-30 14:01 ` Laurent Vivier
0 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2026-07-30 14:01 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi
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++) {
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (4 preceding siblings ...)
2026-07-29 23:19 ` [PATCH v3 5/7] mptsas: do not crash QEMU on migration errors Laurent Vivier
@ 2026-07-29 23:19 ` Laurent Vivier
2026-07-30 13:35 ` Stefan Hajnoczi
2026-07-29 23:19 ` [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors Laurent Vivier
` (2 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:19 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
Laurent Vivier, qemu-stable
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
2026-07-29 23:19 ` [PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
@ 2026-07-30 13:35 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2026-07-30 13:35 UTC (permalink / raw)
To: Laurent Vivier
Cc: qemu-devel, Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, qemu-stable
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (5 preceding siblings ...)
2026-07-29 23:19 ` [PATCH v3 6/7] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
@ 2026-07-29 23:19 ` Laurent Vivier
2026-07-30 13:36 ` Stefan Hajnoczi
2026-07-30 9:20 ` [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
2026-07-30 10:49 ` Michael S. Tsirkin
8 siblings, 1 reply; 14+ messages in thread
From: Laurent Vivier @ 2026-07-29 23:19 UTC (permalink / raw)
To: qemu-devel
Cc: Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi,
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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors
2026-07-29 23:19 ` [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors Laurent Vivier
@ 2026-07-30 13:36 ` Stefan Hajnoczi
0 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2026-07-30 13:36 UTC (permalink / raw)
To: Laurent Vivier
Cc: qemu-devel, Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Michael S. Tsirkin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/7] Harden virtio migration load paths against crafted streams
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (6 preceding siblings ...)
2026-07-29 23:19 ` [PATCH v3 7/7] hw/scsi/spapr_vscsi: do not crash QEMU on migration errors Laurent Vivier
@ 2026-07-30 9:20 ` Michael S. Tsirkin
2026-07-30 10:49 ` Michael S. Tsirkin
8 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2026-07-30 9:20 UTC (permalink / raw)
To: Laurent Vivier
Cc: qemu-devel, Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 0/7] Harden virtio migration load paths against crafted streams
2026-07-29 23:18 [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Laurent Vivier
` (7 preceding siblings ...)
2026-07-30 9:20 ` [PATCH v3 0/7] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
@ 2026-07-30 10:49 ` Michael S. Tsirkin
8 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2026-07-30 10:49 UTC (permalink / raw)
To: Laurent Vivier
Cc: qemu-devel, Amit Shah, qemu-ppc, Harsh Prateek Bora, Hanna Reitz,
Nicholas Piggin, Kevin Wolf, Paolo Bonzini,
Marc-André Lureau, Fam Zheng, qemu-block, Stefan Hajnoczi
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
^ permalink raw reply [flat|nested] 14+ messages in thread