On 2026/07/08 0:25, Michael Tokarev wrote:
(Resending with correct new address of Akihiko Odaki.
Please excuse me for double post).
On 05.07.2026 17:36, Michael S. Tsirkin wrote:
From: Junjie Cao <[email protected]>
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 corrupted save file or 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.
Factor the validation into virtio_net_rss_indirections_len_valid()
and call it from both virtio_net_handle_rss() and
virtio_net_rss_post_load().
Fixes: e41b711485e5 ("virtio-net: add migration support for RSS and
hash report")
Cc: [email protected]
...> @@ -3427,6 +3427,13 @@ static int virtio_net_rss_post_load(void
*opaque, int version_id)
n->rss_data.supported_hash_types =
VIRTIO_NET_RSS_SUPPORTED_HASHES;
}
+ if (!virtio_net_rss_indirections_len_valid(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;
}
Hi!
I tried to pick this one up for 10.0.x stable qemu (LTS) series, and
found out that this series lacks virtio_net_rss_post_load() function
entirely, because it lacks commit 7b6e7e4990 "virtio-net: Retrieve peer
hashing capability" (Cc'ing Akihiko Odaki), and 2 previous commits:
7b6e7e4990 virtio-net: Retrieve peer hashing capability
2deec9ab7d virtio-net: Move virtio_net_get_features() down
14f521f491 net/vhost-vdpa: Report hashing capability
I can introduce new virtio_net_rss_post_load() with just indirection
verification from this patch. But it looks like I can pick up that
commit for 10.0.x too, and make whole thing closer to what we have in
current master.
It does not seem to change machine types/properties and migration
stream.
What do you think?
I would not pull the peer hashing capability series into 10.0.x just to
backport this.
It is not only a refactoring dependency. Commit 7b6e7e49905d
("virtio-net: Retrieve peer hashing capability") bumps the virtio-net
RSS subsection from version 1 to 2 and adds supported_hash_types to the
migration stream. It also changes the guest-visible supported_hash_types
value for vhost-vdpa based on backend capability. That is broader than
the RSS validation change and is not migration-stream neutral.
For 10.0.x, the safer backport is to add a small
virtio_net_rss_post_load() to the existing version-1 RSS subsection and
keep the subsection version unchanged. The hook only needs to reject
restored indirections_len values that the command path cannot produce:
zero, values above VIRTIO_NET_RSS_MAX_TABLE_LEN, or non-powers of two.
That fixes the crash without pulling in the vhost-vdpa hash capability
behavior change.
Regards,
Akihiko Odaki
