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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|