[PATCH] virtio-net: validate RSS indirections_len in post_load

[PATCH] virtio-net: validate RSS indirections_len in post_load

[Junjie Cao]

virtio_net_handle_rss() enforces that indirections_len is a non-zero
power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
virtio_net_rss_post_load() applies none of these checks to values
restored from the migration stream.

A crafted migration stream can set indirections_len to 0.  Even if it
also clears redirect, virtio_load() calls set_features_nocheck() after
the device vmstate (including the RSS subsection and its post_load) has
already been loaded, re-deriving redirect from the negotiated guest
features.  When VIRTIO_NET_F_RSS was negotiated, redirect is set back
to true regardless of the migration stream value.  The receive path
then computes

    hash & (indirections_len - 1)   /* wraps to 0xFFFFFFFF via int promotion */

and uses the result to index into indirections_table, which was not
allocated by the VMState loader when the element count is zero (see
vmstate_handle_alloc()), resulting in a NULL pointer dereference that
crashes QEMU:

  #0  virtio_net_process_rss    ../hw/net/virtio-net.c:1901
  #1  virtio_net_receive_rcu    ../hw/net/virtio-net.c:1921
  #2  virtio_net_do_receive     ../hw/net/virtio-net.c:2061
  #3  nc_sendv_compat           ../net/net.c:823
  #4  qemu_deliver_packet_iov   ../net/net.c:870

The RSS subsection is only loaded when rss_data.enabled is true (via
virtio_net_rss_needed()), and the command path always produces
indirections_len in {1, 2, 4, …, 128}, so an unconditional check
cannot reject a legitimate migration stream.

Fixes: e41b711485e5 ("virtio-net: add migration support for RSS and hash report")
Cc: qemu-stable@nongnu.org
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
 hw/net/virtio-net.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 2a5d642a64..3038836098 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3427,6 +3427,15 @@ static int virtio_net_rss_post_load(void *opaque, int version_id)
         n->rss_data.supported_hash_types = VIRTIO_NET_RSS_SUPPORTED_HASHES;
     }
 
+    if (n->rss_data.indirections_len == 0 ||
+        n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN ||
+        !is_power_of_2(n->rss_data.indirections_len)) {
+        error_report("virtio-net: saved image has invalid RSS "
+                     "indirections_len: %u",
+                     n->rss_data.indirections_len);
+        return -EINVAL;
+    }
+
     return 0;
 }
 
-- 
2.43.0

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Daniel P. Berrangé]

On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> virtio_net_handle_rss() enforces that indirections_len is a non-zero
> power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> virtio_net_rss_post_load() applies none of these checks to values
> restored from the migration stream.
> 
> A crafted migration stream can set indirections_len to 0.  Even if it

The migration stream originating from the source QEMU is trusted.

Is there a problem you can demonstrate with regular QEMU commands
/ versions, not crafting a malicious migration stream.

> also clears redirect, virtio_load() calls set_features_nocheck() after
> the device vmstate (including the RSS subsection and its post_load) has
> already been loaded, re-deriving redirect from the negotiated guest
> features.  When VIRTIO_NET_F_RSS was negotiated, redirect is set back
> to true regardless of the migration stream value.  The receive path
> then computes
> 
>     hash & (indirections_len - 1)   /* wraps to 0xFFFFFFFF via int promotion */
> 
> and uses the result to index into indirections_table, which was not
> allocated by the VMState loader when the element count is zero (see
> vmstate_handle_alloc()), resulting in a NULL pointer dereference that
> crashes QEMU:
> 
>   #0  virtio_net_process_rss    ../hw/net/virtio-net.c:1901
>   #1  virtio_net_receive_rcu    ../hw/net/virtio-net.c:1921
>   #2  virtio_net_do_receive     ../hw/net/virtio-net.c:2061
>   #3  nc_sendv_compat           ../net/net.c:823
>   #4  qemu_deliver_packet_iov   ../net/net.c:870
> 
> The RSS subsection is only loaded when rss_data.enabled is true (via
> virtio_net_rss_needed()), and the command path always produces
> indirections_len in {1, 2, 4, …, 128}, so an unconditional check
> cannot reject a legitimate migration stream.
> 
> Fixes: e41b711485e5 ("virtio-net: add migration support for RSS and hash report")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>
> ---
>  hw/net/virtio-net.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 2a5d642a64..3038836098 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3427,6 +3427,15 @@ static int virtio_net_rss_post_load(void *opaque, int version_id)
>          n->rss_data.supported_hash_types = VIRTIO_NET_RSS_SUPPORTED_HASHES;
>      }
>  
> +    if (n->rss_data.indirections_len == 0 ||
> +        n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN ||
> +        !is_power_of_2(n->rss_data.indirections_len)) {
> +        error_report("virtio-net: saved image has invalid RSS "
> +                     "indirections_len: %u",
> +                     n->rss_data.indirections_len);
> +        return -EINVAL;
> +    }
> +
>      return 0;
>  }
>  
> -- 
> 2.43.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Peter Maydell]

On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > virtio_net_rss_post_load() applies none of these checks to values
> > restored from the migration stream.
> >
> > A crafted migration stream can set indirections_len to 0.  Even if it
>
> The migration stream originating from the source QEMU is trusted.

Is it? In https://www.qemu.org/docs/master/system/security.html we say:

# The following entities are untrusted, meaning that they may be buggy
# or malicious:

#  * Guest
#  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
#  * Network protocols (e.g. NBD, live migration)
#  * User-supplied files (e.g. disk images, kernels, device trees)
#  * Passthrough devices (e.g. PCI, USB)

which explicitly lists "live migration" as an untrusted entity.

I would definitely be extremely cautious about having a threat
model where I had to distrust inbound migration data, but the
above does suggest we aim to handle that, and we have I think
in the past taken patches which add sanity-checking to the
migration data.

thanks
-- PMM

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Michael S. Tsirkin]

On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > virtio_net_rss_post_load() applies none of these checks to values
> > > restored from the migration stream.
> > >
> > > A crafted migration stream can set indirections_len to 0.  Even if it
> >
> > The migration stream originating from the source QEMU is trusted.
> 
> Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> 
> # The following entities are untrusted, meaning that they may be buggy
> # or malicious:
> 
> #  * Guest
> #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> #  * Network protocols (e.g. NBD, live migration)
> #  * User-supplied files (e.g. disk images, kernels, device trees)
> #  * Passthrough devices (e.g. PCI, USB)
> 
> which explicitly lists "live migration" as an untrusted entity.
> 
> I would definitely be extremely cautious about having a threat
> model where I had to distrust inbound migration data, but the
> above does suggest we aim to handle that, and we have I think
> in the past taken patches which add sanity-checking to the
> migration data.
> 
> thanks
> -- PMM


And we even assigned a low priority CVEs to these.

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Daniel P. Berrangé]

On Mon, Mar 23, 2026 at 09:57:24AM -0400, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > > virtio_net_rss_post_load() applies none of these checks to values
> > > > restored from the migration stream.
> > > >
> > > > A crafted migration stream can set indirections_len to 0.  Even if it
> > >
> > > The migration stream originating from the source QEMU is trusted.
> > 
> > Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> > 
> > # The following entities are untrusted, meaning that they may be buggy
> > # or malicious:
> > 
> > #  * Guest
> > #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> > #  * Network protocols (e.g. NBD, live migration)
> > #  * User-supplied files (e.g. disk images, kernels, device trees)
> > #  * Passthrough devices (e.g. PCI, USB)
> > 
> > which explicitly lists "live migration" as an untrusted entity.
> > 
> > I would definitely be extremely cautious about having a threat
> > model where I had to distrust inbound migration data, but the
> > above does suggest we aim to handle that, and we have I think
> > in the past taken patches which add sanity-checking to the
> > migration data.
> 
> And we even assigned a low priority CVEs to these.

Do you have an example of that ?

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Peter Maydell]

On Mon, 23 Mar 2026 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 23, 2026 at 09:57:24AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > > On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > > > virtio_net_rss_post_load() applies none of these checks to values
> > > > > restored from the migration stream.
> > > > >
> > > > > A crafted migration stream can set indirections_len to 0.  Even if it
> > > >
> > > > The migration stream originating from the source QEMU is trusted.
> > >
> > > Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> > >
> > > # The following entities are untrusted, meaning that they may be buggy
> > > # or malicious:
> > >
> > > #  * Guest
> > > #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> > > #  * Network protocols (e.g. NBD, live migration)
> > > #  * User-supplied files (e.g. disk images, kernels, device trees)
> > > #  * Passthrough devices (e.g. PCI, USB)
> > >
> > > which explicitly lists "live migration" as an untrusted entity.
> > >
> > > I would definitely be extremely cautious about having a threat
> > > model where I had to distrust inbound migration data, but the
> > > above does suggest we aim to handle that, and we have I think
> > > in the past taken patches which add sanity-checking to the
> > > migration data.
> >
> > And we even assigned a low priority CVEs to these.
>
> Do you have an example of that ?

Here's one from last year:
https://access.redhat.com/security/cve/cve-2025-54566

I vaguely recall we had a set of them some years ago too
(probably somebody specifically looking for flaws in the
category). https://access.redhat.com/security/cve/cve-2013-4536
might be an example of that.

-- PMM

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Daniel P. Berrangé]

On Mon, Mar 23, 2026 at 03:25:10PM +0000, Peter Maydell wrote:
> On Mon, 23 Mar 2026 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 09:57:24AM -0400, Michael S. Tsirkin wrote:
> > > On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > > > On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > >
> > > > > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > > > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > > > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > > > > virtio_net_rss_post_load() applies none of these checks to values
> > > > > > restored from the migration stream.
> > > > > >
> > > > > > A crafted migration stream can set indirections_len to 0.  Even if it
> > > > >
> > > > > The migration stream originating from the source QEMU is trusted.
> > > >
> > > > Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> > > >
> > > > # The following entities are untrusted, meaning that they may be buggy
> > > > # or malicious:
> > > >
> > > > #  * Guest
> > > > #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> > > > #  * Network protocols (e.g. NBD, live migration)
> > > > #  * User-supplied files (e.g. disk images, kernels, device trees)
> > > > #  * Passthrough devices (e.g. PCI, USB)
> > > >
> > > > which explicitly lists "live migration" as an untrusted entity.
> > > >
> > > > I would definitely be extremely cautious about having a threat
> > > > model where I had to distrust inbound migration data, but the
> > > > above does suggest we aim to handle that, and we have I think
> > > > in the past taken patches which add sanity-checking to the
> > > > migration data.
> > >
> > > And we even assigned a low priority CVEs to these.
> >
> > Do you have an example of that ?
> 
> Here's one from last year:
> https://access.redhat.com/security/cve/cve-2025-54566

I think this one is valid, because it involves incorrect handling
of settings that are controlled by the guest OS. There's no
external party claimed to be modifying the migration stream IIUC

> I vaguely recall we had a set of them some years ago too
> (probably somebody specifically looking for flaws in the
> category). https://access.redhat.com/security/cve/cve-2013-4536
> might be an example of that.

I think this one is probably issued in error.

If we're considering that the migration data stream is modifiable
by an attacker, the implication is that the attacker has arbitrary
read and write over the entire of guest RAM. That in turn implies
no guest OS can ever be trusted after a migration has been performed,

That is certainly not the case though.

We establish trust in the guest RAM after migration by protecting
the live migration data stream with TLS (or an equivalent mechanism
external to QEMU), and including some mechanism for authenticating
the connections. We prove the incoming connection is from the
expected source QEMU. That protection also applies to the VMstate
data.

So the only entity would can give us malicious vmstate data would
be the source QEMU.

The risk would appear to be that the source QEMU has been compromised,
but has been unable to break out into the source host OS. The migration
data would thus be leveraged as a way to break out into the target host
OS instead.

This feels dubious though. The source QEMU and target QEMU would be
assumed to be using the same security facilities (running non-root,
using seccomp, using selinux/apparmor, using namespaces, etc). So if
it could not break out of QEMU on the source host, I can't see that
migration would make it possible todo that on the target host either.


The save/restore of VM state to/from disk, for the purpose of VM
snapshotting is slightly different as  we don't have a network
channel involved.

None the less, I'd claim that the saved snapshot file must be considered
trusted, because it again contains data (guest RAM) that we inherently
must trust. If an attacker has the ability to modify a saved state file,
then we've already lost all trust.


I can see scope for CVEs in QEMU wrt vmstate, if:

 * a malicious guest OS driver is able to configure some aspect
   of a virtual device, such that it then causes misbehaviour in
   later handling of the VM state.

   IIUC the recent cve-2025-54566 is an example of that


 * untrusted data being processed by the virtual device, causes
   the device to get itself into a state which then causes
   misbehaviour in handling of VM state. 


I don't accept scope for CVEs in QEMU for an external attacker modifying
the migration data stream or saved state file contents though. AFAICS,
such possibilities imply gross misconfiguration of QEMU by the mgmt app,
and should be CVEs in the mgmt app instead.



With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Peter Maydell]

On Mon, 23 Mar 2026 at 15:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 23, 2026 at 03:25:10PM +0000, Peter Maydell wrote:
> > On Mon, 23 Mar 2026 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Mon, Mar 23, 2026 at 09:57:24AM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > > > > On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > > > > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > > > > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > > > > > virtio_net_rss_post_load() applies none of these checks to values
> > > > > > > restored from the migration stream.
> > > > > > >
> > > > > > > A crafted migration stream can set indirections_len to 0.  Even if it
> > > > > >
> > > > > > The migration stream originating from the source QEMU is trusted.
> > > > >
> > > > > Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> > > > >
> > > > > # The following entities are untrusted, meaning that they may be buggy
> > > > > # or malicious:
> > > > >
> > > > > #  * Guest
> > > > > #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> > > > > #  * Network protocols (e.g. NBD, live migration)
> > > > > #  * User-supplied files (e.g. disk images, kernels, device trees)
> > > > > #  * Passthrough devices (e.g. PCI, USB)
> > > > >
> > > > > which explicitly lists "live migration" as an untrusted entity.
> > > > >
> > > > > I would definitely be extremely cautious about having a threat
> > > > > model where I had to distrust inbound migration data, but the
> > > > > above does suggest we aim to handle that, and we have I think
> > > > > in the past taken patches which add sanity-checking to the
> > > > > migration data.
> > > >
> > > > And we even assigned a low priority CVEs to these.
> > >
> > > Do you have an example of that ?
> >
> > Here's one from last year:
> > https://access.redhat.com/security/cve/cve-2025-54566
>
> I think this one is valid, because it involves incorrect handling
> of settings that are controlled by the guest OS. There's no
> external party claimed to be modifying the migration stream IIUC

I think the "guest OS sets things wrongly" is CVE-2025-54567,
and 54566 is specifically for the "migration stream is malicious"
case. The commit fixing them:
https://gitlab.com/qemu-project/qemu/-/commit/cad9aa6fbdccd95e56e10cfa57c354a20a333717
fixes both at the same time, by providing a validation function
and calling it both (a) when the guest OS writes the settings
and (b) in post_load. If we trust migration data then we don't need
to validate it in post-load, we could just say "make sure your
source QEMU has the fix for (a) and that you've restarted the guest".

> > I vaguely recall we had a set of them some years ago too
> > (probably somebody specifically looking for flaws in the
> > category). https://access.redhat.com/security/cve/cve-2013-4536
> > might be an example of that.
>
> I think this one is probably issued in error.
>
> If we're considering that the migration data stream is modifiable
> by an attacker, the implication is that the attacker has arbitrary
> read and write over the entire of guest RAM. That in turn implies
> no guest OS can ever be trusted after a migration has been performed,

> That is certainly not the case though.

Trusted by whom? If I'm a cloud vendor then I don't trust the
guest OS in the first place. QEMU itself should consider everything
in guest RAM untrusted data, because it's guest-controlled.
You can probably have setups where the user who owns the VM might
be able to modify the migration stream but no third party can
(e.g. if you give them the ability to vmsave/vmload to a file
that they own), in the same way you can give the VM owner the
ability to directly write to the disk images the VM is using
without that being a way for them to escape to the host.

> I don't accept scope for CVEs in QEMU for an external attacker modifying
> the migration data stream or saved state file contents though. AFAICS,
> such possibilities imply gross misconfiguration of QEMU by the mgmt app,
> and should be CVEs in the mgmt app instead.

I agree that you have a pretty weird threat/operating model if
you allow the migration-stream to be considered untrusted, because
it's reasonable and sensible to secure it. But I have a vague
recollection that this language is in the doc precisely because
at least one person/party has argued in the past in favour of
treating the migration stream as on the security boundary. I don't
object to our deciding we want to call the migration-stream trusted --
but I do think this is a policy change from our current stance.

Paolo, Stefan, do you happen to remember anything about this ?

thanks
-- PMM

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Daniel P. Berrangé]

On Mon, Mar 23, 2026 at 04:15:56PM +0000, Peter Maydell wrote:
> On Mon, 23 Mar 2026 at 15:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 03:25:10PM +0000, Peter Maydell wrote:
> > > On Mon, 23 Mar 2026 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > On Mon, Mar 23, 2026 at 09:57:24AM -0400, Michael S. Tsirkin wrote:
> > > > > On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > > > > > On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > > > >
> > > > > > > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > > > > > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > > > > > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > > > > > > virtio_net_rss_post_load() applies none of these checks to values
> > > > > > > > restored from the migration stream.
> > > > > > > >
> > > > > > > > A crafted migration stream can set indirections_len to 0.  Even if it
> > > > > > >
> > > > > > > The migration stream originating from the source QEMU is trusted.
> > > > > >
> > > > > > Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> > > > > >
> > > > > > # The following entities are untrusted, meaning that they may be buggy
> > > > > > # or malicious:
> > > > > >
> > > > > > #  * Guest
> > > > > > #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> > > > > > #  * Network protocols (e.g. NBD, live migration)
> > > > > > #  * User-supplied files (e.g. disk images, kernels, device trees)
> > > > > > #  * Passthrough devices (e.g. PCI, USB)
> > > > > >
> > > > > > which explicitly lists "live migration" as an untrusted entity.
> > > > > >
> > > > > > I would definitely be extremely cautious about having a threat
> > > > > > model where I had to distrust inbound migration data, but the
> > > > > > above does suggest we aim to handle that, and we have I think
> > > > > > in the past taken patches which add sanity-checking to the
> > > > > > migration data.
> > > > >
> > > > > And we even assigned a low priority CVEs to these.
> > > >
> > > > Do you have an example of that ?
> > >
> > > Here's one from last year:
> > > https://access.redhat.com/security/cve/cve-2025-54566
> >
> > I think this one is valid, because it involves incorrect handling
> > of settings that are controlled by the guest OS. There's no
> > external party claimed to be modifying the migration stream IIUC
> 
> I think the "guest OS sets things wrongly" is CVE-2025-54567,
> and 54566 is specifically for the "migration stream is malicious"
> case. The commit fixing them:
> https://gitlab.com/qemu-project/qemu/-/commit/cad9aa6fbdccd95e56e10cfa57c354a20a333717
> fixes both at the same time, by providing a validation function
> and calling it both (a) when the guest OS writes the settings
> and (b) in post_load. If we trust migration data then we don't need
> to validate it in post-load, we could just say "make sure your
> source QEMU has the fix for (a) and that you've restarted the guest".

Ah, yes, the pairing of CVEs is key here.

Our users expect their workloads to execute without interruption.
Thus our story for fixing CVEs bugs without guest impact is to
live migrate the guest onto the fixed CVE.

So in this case, I think we do need the fix in post_load as a
way to mitigate the root cause CVE in existing workloads that
cannot be restarted. 

> > > I vaguely recall we had a set of them some years ago too
> > > (probably somebody specifically looking for flaws in the
> > > category). https://access.redhat.com/security/cve/cve-2013-4536
> > > might be an example of that.
> >
> > I think this one is probably issued in error.
> >
> > If we're considering that the migration data stream is modifiable
> > by an attacker, the implication is that the attacker has arbitrary
> > read and write over the entire of guest RAM. That in turn implies
> > no guest OS can ever be trusted after a migration has been performed,
> 
> > That is certainly not the case though.
> 
> Trusted by whom? If I'm a cloud vendor then I don't trust the
> guest OS in the first place. QEMU itself should consider everything
> in guest RAM untrusted data, because it's guest-controlled.
> You can probably have setups where the user who owns the VM might
> be able to modify the migration stream but no third party can
> (e.g. if you give them the ability to vmsave/vmload to a file
> that they own), in the same way you can give the VM owner the
> ability to directly write to the disk images the VM is using
> without that being a way for them to escape to the host.

Hmm, yes, load/save into a file that the guest owner controls
is a good point. I'm fairly sure I recall that OpenStack allows
for exactly that scenario. So the host owner would need to
treat the vmstate as potentially hostile in that case.

IOW, bugs that affect handling of vm state that merely result
in an assert are merely self-inflicted DoS by the guest owner.
Bugs that could be leveraged to exploit QEMU itself should be
CVE worthy.

So even if the migration stream can be considerd trusted, we
can't trust vmstate in general due to the load/save scenario.

This is probably a case where we ought to have had a host
controlled digital signature over vmstate data to validate
its integrity.


> > I don't accept scope for CVEs in QEMU for an external attacker modifying
> > the migration data stream or saved state file contents though. AFAICS,
> > such possibilities imply gross misconfiguration of QEMU by the mgmt app,
> > and should be CVEs in the mgmt app instead.
> 
> I agree that you have a pretty weird threat/operating model if
> you allow the migration-stream to be considered untrusted, because
> it's reasonable and sensible to secure it. But I have a vague
> recollection that this language is in the doc precisely because
> at least one person/party has argued in the past in favour of
> treating the migration stream as on the security boundary. I don't
> object to our deciding we want to call the migration-stream trusted --
> but I do think this is a policy change from our current stance.

Possibly we should clarify the language to say that the vmstate
format is untrusted due to the load/save possibility to guest
owner controlled storage.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Daniel P. Berrangé]

On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> On Mon, 23 Mar 2026 at 13:43, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> > > virtio_net_handle_rss() enforces that indirections_len is a non-zero
> > > power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> > > virtio_net_rss_post_load() applies none of these checks to values
> > > restored from the migration stream.
> > >
> > > A crafted migration stream can set indirections_len to 0.  Even if it
> >
> > The migration stream originating from the source QEMU is trusted.
> 
> Is it? In https://www.qemu.org/docs/master/system/security.html we say:
> 
> # The following entities are untrusted, meaning that they may be buggy
> # or malicious:
> 
> #  * Guest
> #  * User-facing interfaces (e.g. VNC, SPICE, WebSocket)
> #  * Network protocols (e.g. NBD, live migration)
> #  * User-supplied files (e.g. disk images, kernels, device trees)
> #  * Passthrough devices (e.g. PCI, USB)
> 
> which explicitly lists "live migration" as an untrusted entity.
> 
> I would definitely be extremely cautious about having a threat
> model where I had to distrust inbound migration data, but the
> above does suggest we aim to handle that, and we have I think
> in the past taken patches which add sanity-checking to the
> migration data.

My view of the migration stream is that we authenticate the client
at the point of connection (either explicitly with SASL, or implicitly
with a x509 certificate validation), and protect the data stream
integrity with TLS, or equivalent.

For the vmstate data, we simply expect that to reflect the current
QEMU configuration, and variation of that is liable to lead to
a crash or worse.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Peter Maydell]

On Mon, 23 Mar 2026 at 14:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 23, 2026 at 01:53:53PM +0000, Peter Maydell wrote:
> > I would definitely be extremely cautious about having a threat
> > model where I had to distrust inbound migration data, but the
> > above does suggest we aim to handle that, and we have I think
> > in the past taken patches which add sanity-checking to the
> > migration data.
>
> My view of the migration stream is that we authenticate the client
> at the point of connection (either explicitly with SASL, or implicitly
> with a x509 certificate validation), and protect the data stream
> integrity with TLS, or equivalent.

This seems like it would be useful clarifying information to
add to the security.rst document under the "Sensitive configurations"
subsection as part of documenting what we recommend, even if we
decide that we want to continue treating malicious migration-data
as part of our threat model / security boundary.

-- PMM

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Michael S. Tsirkin]

On Mon, Mar 23, 2026 at 09:15:31PM +0800, Junjie Cao wrote:
> virtio_net_handle_rss() enforces that indirections_len is a non-zero
> power of two no larger than VIRTIO_NET_RSS_MAX_TABLE_LEN, but
> virtio_net_rss_post_load() applies none of these checks to values
> restored from the migration stream.
> 
> A crafted migration stream can set indirections_len to 0.  Even if it
> also clears redirect, virtio_load() calls set_features_nocheck() after
> the device vmstate (including the RSS subsection and its post_load) has
> already been loaded, re-deriving redirect from the negotiated guest
> features.  When VIRTIO_NET_F_RSS was negotiated, redirect is set back
> to true regardless of the migration stream value.  The receive path
> then computes
> 
>     hash & (indirections_len - 1)   /* wraps to 0xFFFFFFFF via int promotion */
> 
> and uses the result to index into indirections_table, which was not
> allocated by the VMState loader when the element count is zero (see
> vmstate_handle_alloc()), resulting in a NULL pointer dereference that
> crashes QEMU:
> 
>   #0  virtio_net_process_rss    ../hw/net/virtio-net.c:1901
>   #1  virtio_net_receive_rcu    ../hw/net/virtio-net.c:1921
>   #2  virtio_net_do_receive     ../hw/net/virtio-net.c:2061
>   #3  nc_sendv_compat           ../net/net.c:823
>   #4  qemu_deliver_packet_iov   ../net/net.c:870
> 
> The RSS subsection is only loaded when rss_data.enabled is true (via
> virtio_net_rss_needed()), and the command path always produces
> indirections_len in {1, 2, 4, …, 128}, so an unconditional check
> cannot reject a legitimate migration stream.
> 
> Fixes: e41b711485e5 ("virtio-net: add migration support for RSS and hash report")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>


Thanks for the patch!
Yet something to improve:

> ---
>  hw/net/virtio-net.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 2a5d642a64..3038836098 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -3427,6 +3427,15 @@ static int virtio_net_rss_post_load(void *opaque, int version_id)
>          n->rss_data.supported_hash_types = VIRTIO_NET_RSS_SUPPORTED_HASHES;
>      }
>  
> +    if (n->rss_data.indirections_len == 0 ||


not needed since it's included in is_power_of_2 check below?

> +        n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN ||
> +        !is_power_of_2(n->rss_data.indirections_len)) {

with == 0 removed, the logic is duplicated from
virtio_net_device_realize.
How about factoring it out to a helper?

> +        error_report("virtio-net: saved image has invalid RSS "
> +                     "indirections_len: %u",
> +                     n->rss_data.indirections_len);
> +        return -EINVAL;
> +    }
> +
>      return 0;
>  }
>  
> -- 
> 2.43.0

Re: [PATCH] virtio-net: validate RSS indirections_len in post_load

[Junjie Cao]

Thanks everyone for the discussion, very rewarding for me.

I've sent v2, addressing Michael's review.  I also tightened the
commit message to scope the trigger explicitly to corrupted save
files and crafted migration streams.

Thanks,
Junjie