Re: [PATCH net-next v5 2/9] virtio-net: zero the whole tx header

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] virtio_net: uninitialized skb->cb read in
  virtnet_receive_done() [1]
--

commit 539d0b476e95c38e08289348fc9e76ffb578e50d
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

virtio-net: zero the whole tx header

This commit zeroes the entire negotiated virtio-net transmit header in
xmit_skb() before populating it so that uninitialized skb->cb or headroom
bytes are not sent to the device.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index dd2c41ef6fbe7..4c740b403ad1e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3326,14 +3326,16 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)
>  	else
>  		hdr = &skb_vnet_common_hdr(skb)->tnl_hdr;
>  
> +	/* Fields the helper below does not fill in must not carry stale
> +	 * skb->cb or headroom bytes to the device.
> +	 */
> +	memset(hdr, 0, hdr_len);
> +
>  	if (virtio_net_hdr_tnl_from_skb(skb, hdr, vi->tx_tnl,
>  					virtio_is_little_endian(vi->vdev), 0,
>  					false, feature_hdrlen))
>  		return -EPROTO;
>  
> -	if (vi->mergeable_rx_bufs)
> -		hdr->hash_hdr.hdr.num_buffers = 0;
> -

[Severity: Critical]
This problem wasn't introduced by this patch, but can virtnet_receive_done()
read uninitialized skb->cb fields [1] when an XDP program returns
XDP_PASS?

In receive_small_xdp() and receive_mergeable_xdp(), the skb constructed on
XDP_PASS via virtnet_build_skb() or build_skb_from_xdp_buff() does not copy
or zero the virtio-net header in skb_vnet_common_hdr(skb).  When
receive_buf() passes that skb to virtnet_receive_done(), only hdr->hdr.flags
is written before the rest of hdr is read:

drivers/net/virtio_net.c:virtnet_receive_done() {
	...
	hdr = skb_vnet_common_hdr(skb);
	if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
		virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);

	hdr->hdr.flags = flags;
	if (virtio_net_handle_csum_offload(skb, &hdr->hdr, vi->rx_tnl_csum)) {
	...
}

virtio_net_handle_csum_offload() and virtio_skb_set_hash() then evaluate
garbage csum_start, csum_offset, gso_type, and hash metadata from skb->cb,
causing corrupted checksums, unexpected packet drops, or bogus skb state.

[1] https://sashiko.dev/#/bug/linux-89f94565-4be5-4c5d-b3a1-d88fa369641f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-v6-7-topic-virtio-net-ptp-v5-0-7ab67b572f4f@pengutronix.de?part=2