All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Harden virtio migration load paths against crafted streams
@ 2026-07-10 10:28 Laurent Vivier
  2026-07-10 10:28 ` [PATCH 1/3] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Laurent Vivier @ 2026-07-10 10:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Hanna Reitz, Fam Zheng, qemu-block,
	Michael S. Tsirkin, Kevin Wolf, Laurent Vivier, Stefan Hajnoczi,
	Paolo Bonzini, Amit Shah

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, and assert()/exit(1) calls in
virtio-scsi request loading.

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 validates the virtio-serial nr_active_ports count against the
configured maximum before allocating the post-load array.

Patch 2 replaces the assertions in qemu_get_virtqueue_element() with
a bounds check returning NULL, and updates all callers (virtio-serial,
virtio-blk, virtio-scsi, scsi-bus) to handle the failure gracefully.

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

Tested with migration round-trips for virtio-serial (0 to 511 ports),
virtio-blk (1-2 disks), and virtio-scsi (1-2 disks), plus the
original PoC reproducers for issues #3801 and #3802.

Laurent Vivier (3):
  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/block/virtio-blk.c       |  4 ++++
 hw/char/virtio-serial-bus.c |  7 +++++++
 hw/scsi/scsi-bus.c          |  4 ++++
 hw/scsi/virtio-scsi.c       | 20 ++++++++++++++++----
 hw/virtio/virtio.c          | 11 ++++-------
 5 files changed, 35 insertions(+), 11 deletions(-)

-- 
2.54.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream
  2026-07-10 10:28 [PATCH 0/3] Harden virtio migration load paths against crafted streams Laurent Vivier
@ 2026-07-10 10:28 ` Laurent Vivier
  2026-07-10 10:28 ` [PATCH 2/3] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state Laurent Vivier
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2026-07-10 10:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Hanna Reitz, Fam Zheng, qemu-block,
	Michael S. Tsirkin, Kevin Wolf, Laurent Vivier, Stefan Hajnoczi,
	Paolo Bonzini, Amit Shah, 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>
---
 hw/char/virtio-serial-bus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c1973f0248fc..80f1b308aa53 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
 
     qemu_get_be32s(f, &nr_active_ports);
 
+    if (nr_active_ports > max_nr_ports) {
+        return -EINVAL;
+    }
+
     if (nr_active_ports) {
         ret = fetch_active_ports_list(f, s, nr_active_ports);
         if (ret) {
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state
  2026-07-10 10:28 [PATCH 0/3] Harden virtio migration load paths against crafted streams Laurent Vivier
  2026-07-10 10:28 ` [PATCH 1/3] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
@ 2026-07-10 10:28 ` Laurent Vivier
  2026-07-10 10:28 ` [PATCH 3/3] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
  2026-07-25 15:34 ` [PATCH 0/3] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
  3 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2026-07-10 10:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Hanna Reitz, Fam Zheng, qemu-block,
	Michael S. Tsirkin, Kevin Wolf, Laurent Vivier, Stefan Hajnoczi,
	Paolo Bonzini, Amit Shah, 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. Update all callers (virtio-serial-bus, virtio-blk,
virtio-scsi) to handle the NULL return and fail the migration
gracefully.

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>
---
 hw/block/virtio-blk.c       |  4 ++++
 hw/char/virtio-serial-bus.c |  3 +++
 hw/scsi/scsi-bus.c          |  4 ++++
 hw/scsi/virtio-scsi.c       |  4 ++++
 hw/virtio/virtio.c          | 11 ++++-------
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff4c..42c0f2553f18 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1384,6 +1384,10 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
         }
 
         req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
+        if (!req) {
+            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 80f1b308aa53..4ed8840b2efe 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -763,6 +763,9 @@ static int fetch_active_ports_list(QEMUFile *f,
 
             port->elem =
                 qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
+            if (!port->elem) {
+                return -EINVAL;
+            }
 
             /*
              *  Port was throttled on source machine.  Let's
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index dccb2f25b2af..96c4844374c5 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1915,6 +1915,10 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
         req->retry = (sbyte == 1);
         if (bus->info->load_request) {
             req->hba_private = bus->info->load_request(f, req);
+            if (!req->hba_private) {
+                scsi_req_unref(req);
+                return -EINVAL;
+            }
         }
         if (req->ops->load_request) {
             req->ops->load_request(f, req);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 6c7376801190..8dd0b88a30c4 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -274,6 +274,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) {
+        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 f4d86a365530..7bccfdde33e7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2167,13 +2167,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] 6+ messages in thread

* [PATCH 3/3] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream
  2026-07-10 10:28 [PATCH 0/3] Harden virtio migration load paths against crafted streams Laurent Vivier
  2026-07-10 10:28 ` [PATCH 1/3] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
  2026-07-10 10:28 ` [PATCH 2/3] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state Laurent Vivier
@ 2026-07-10 10:28 ` Laurent Vivier
  2026-07-25 15:34 ` [PATCH 0/3] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
  3 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2026-07-10 10:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Hanna Reitz, Fam Zheng, qemu-block,
	Michael S. Tsirkin, Kevin Wolf, Laurent Vivier, Stefan Hajnoczi,
	Paolo Bonzini, Amit Shah, 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>
---
 hw/scsi/virtio-scsi.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 8dd0b88a30c4..b34cda8fd85a 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -271,7 +271,10 @@ 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) {
+        return NULL;
+    }
+
     req = qemu_get_virtqueue_element(vdev, f,
                                      sizeof(VirtIOSCSIReq) + vs->cdb_size);
     if (!req) {
@@ -283,14 +286,19 @@ 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);
+        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) {
+        scsi_req_unref(sreq);
+        virtio_scsi_free_req(req);
+        return NULL;
     }
+
     return req;
 }
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] Harden virtio migration load paths against crafted streams
  2026-07-10 10:28 [PATCH 0/3] Harden virtio migration load paths against crafted streams Laurent Vivier
                   ` (2 preceding siblings ...)
  2026-07-10 10:28 ` [PATCH 3/3] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
@ 2026-07-25 15:34 ` Michael S. Tsirkin
  2026-07-28 15:00   ` Kevin Wolf
  3 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2026-07-25 15:34 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: qemu-devel, Marc-André Lureau, Hanna Reitz, Fam Zheng,
	qemu-block, Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, Amit Shah

On Fri, Jul 10, 2026 at 12:28:56PM +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, and assert()/exit(1) calls in
> virtio-scsi request loading.
> 
> 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 validates the virtio-serial nr_active_ports count against the
> configured maximum before allocating the post-load array.
> 
> Patch 2 replaces the assertions in qemu_get_virtqueue_element() with
> a bounds check returning NULL, and updates all callers (virtio-serial,
> virtio-blk, virtio-scsi, scsi-bus) to handle the failure gracefully.
> 
> Patch 3 replaces the remaining assert() and exit(1) calls in
> virtio_scsi_load_request() with proper error returns.
> 
> Tested with migration round-trips for virtio-serial (0 to 511 ports),
> virtio-blk (1-2 disks), and virtio-scsi (1-2 disks), plus the
> original PoC reproducers for issues #3801 and #3802.
> 
> Laurent Vivier (3):
>   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

Kevin objected to my version of this, I guess same will apply?

>  hw/block/virtio-blk.c       |  4 ++++
>  hw/char/virtio-serial-bus.c |  7 +++++++
>  hw/scsi/scsi-bus.c          |  4 ++++
>  hw/scsi/virtio-scsi.c       | 20 ++++++++++++++++----
>  hw/virtio/virtio.c          | 11 ++++-------
>  5 files changed, 35 insertions(+), 11 deletions(-)


A similar exit in mptsas?

> -- 
> 2.54.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] Harden virtio migration load paths against crafted streams
  2026-07-25 15:34 ` [PATCH 0/3] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
@ 2026-07-28 15:00   ` Kevin Wolf
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2026-07-28 15:00 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Laurent Vivier, qemu-devel, Marc-André Lureau, Hanna Reitz,
	Fam Zheng, qemu-block, Stefan Hajnoczi, Paolo Bonzini, Amit Shah

Am 25.07.2026 um 17:34 hat Michael S. Tsirkin geschrieben:
> On Fri, Jul 10, 2026 at 12:28:56PM +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, and assert()/exit(1) calls in
> > virtio-scsi request loading.
> > 
> > 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 validates the virtio-serial nr_active_ports count against the
> > configured maximum before allocating the post-load array.
> > 
> > Patch 2 replaces the assertions in qemu_get_virtqueue_element() with
> > a bounds check returning NULL, and updates all callers (virtio-serial,
> > virtio-blk, virtio-scsi, scsi-bus) to handle the failure gracefully.
> > 
> > Patch 3 replaces the remaining assert() and exit(1) calls in
> > virtio_scsi_load_request() with proper error returns.
> > 
> > Tested with migration round-trips for virtio-serial (0 to 511 ports),
> > virtio-blk (1-2 disks), and virtio-scsi (1-2 disks), plus the
> > original PoC reproducers for issues #3801 and #3802.
> > 
> > Laurent Vivier (3):
> >   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
> 
> Kevin objected to my version of this, I guess same will apply?

Objecting is a big word. I suggested some improvements around passing
errors to the caller instead of trying to handle everything locally. But
yes, at the first sight I think they would apply here, too.

Kevin



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-28 15:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 10:28 [PATCH 0/3] Harden virtio migration load paths against crafted streams Laurent Vivier
2026-07-10 10:28 ` [PATCH 1/3] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream Laurent Vivier
2026-07-10 10:28 ` [PATCH 2/3] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state Laurent Vivier
2026-07-10 10:28 ` [PATCH 3/3] hw/scsi/virtio-scsi: harden virtio_scsi_load_request() against invalid stream Laurent Vivier
2026-07-25 15:34 ` [PATCH 0/3] Harden virtio migration load paths against crafted streams Michael S. Tsirkin
2026-07-28 15:00   ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.