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

["Michael S. Tsirkin" <mst@redhat.com>]

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