Re: [PATCH RFC v4 5/6] virtio-net: add XDP rx timestamp metadata

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] virtio-net: virtnet_xdp_rx_timestamp() reads header overlapping XDP data_meta
--

commit 232ef11034e0a6c3f2481ceaa1291cc9268f328c
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

virtio-net: add XDP rx timestamp metadata

This commit implements the xmo_rx_timestamp XDP metadata callback in
virtio_net so XDP programs can read hardware receive timestamps via
bpf_xdp_metadata_rx_timestamp(). It also installs virtnet_xdp_metadata_ops
when VIRTIO_NET_F_RX_TSTAMP is negotiated.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 23af10c613b48..e79b65653fad7 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -6848,8 +6848,30 @@ static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash,
>  	return 0;
>  }
>  
> +static int virtnet_xdp_rx_timestamp(const struct xdp_md *_ctx, u64 *timestamp)
> +{
> +	const struct xdp_buff *xdp = (void *)_ctx;
> +	const struct virtio_net_common_hdr *hdr;
> +	struct virtnet_info *vi;
> +	u64 tstamp;
> +
> +	vi = netdev_priv(xdp->rxq->dev);
> +	if (!READ_ONCE(vi->hwts_rx_en))
> +		return -ENODATA;
> +
> +	hdr = (const struct virtio_net_common_hdr *)(xdp->data - vi->hdr_len);
> +	tstamp = get_unaligned_le64(hdr->tstamp);

[Severity: High]
Can reading hdr->tstamp in place from xdp->data - vi->hdr_len collide with
the XDP data_meta region when an XDP program uses bpf_xdp_adjust_meta()?

When VIRTIO_NET_F_RX_TSTAMP is negotiated and hardware rx timestamping is
enabled via virtnet_hwtstamp_set(), virtnet_probe() sets vi->hdr_len to 40
bytes:

drivers/net/virtio_net.c:virtnet_probe() {
	if (vi->has_rx_tstamp)
		vi->hdr_len = offsetofend(struct virtio_net_common_hdr, tstamp);
}

In the XDP receive paths (receive_small_xdp(), receive_mergeable_xdp() ->
virtnet_build_xdp_buff_mrg(), and virtnet_receive_xsk_buf()), the
virtio_net_common_hdr remains in the headroom immediately preceding
xdp->data, spanning [xdp->data - 40, xdp->data). Within that header,
hdr->tstamp sits at [xdp->data - 8, xdp->data) and hdr->hash_v1_hdr sits at
[xdp->data - 28, xdp->data - 20).

Documentation/networking/xdp-rx-metadata.rst notes that drivers must move
device-reserved metadata out of the [xdp->data_meta, xdp->data) area before
running the XDP program because XDP programs call bpf_xdp_adjust_meta()
first and then populate their custom metadata structure in [xdp->data_meta,
xdp->data) using the xdp_metadata kfuncs.

For example, in tools/testing/selftests/bpf/progs/xdp_hw_metadata.c:rx(),
where struct xdp_meta is 32 bytes:

	err = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta));
	if (err)
		return XDP_PASS;

	data = (void *)(long)ctx->data;
	meta = (void *)(long)ctx->data_meta;
	if (meta + 1 > data)
		return XDP_PASS;

	meta->hint_valid = 0;

	meta->xdp_timestamp = bpf_ktime_get_tai_ns();
	err = bpf_xdp_metadata_rx_timestamp(ctx, &meta->rx_timestamp);
	if (err)
		meta->rx_timestamp_err = err;
	else
		meta->hint_valid |= XDP_META_FIELD_TS;

	err = bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash,
				       &meta->rx_hash_type);

Here, initializing meta->hint_valid = 0 writes to [xdp->data - 4,
xdp->data), which overwrites the upper 32 bits of hdr->tstamp before
bpf_xdp_metadata_rx_timestamp() calls virtnet_xdp_rx_timestamp(). Then,
writing meta->xdp_timestamp and meta->rx_timestamp at [xdp->data - 32,
xdp->data - 16) overwrites hdr->hash_v1_hdr at [xdp->data - 28,
xdp->data - 22) before bpf_xdp_metadata_rx_hash() calls
virtnet_xdp_rx_hash().

Unlike the skb paths (receive_buf() and virtnet_receive_xsk_buf()), which
save the timestamp via virtnet_record_rx_tstamp() before invoking the XDP
program, virtnet_xdp_rx_timestamp() reads from the live buffer during XDP
execution. Should the driver extract or preserve the rx metadata outside
the [xdp->data_meta, xdp->data) region before running the XDP program?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-v6-7-topic-virtio-net-ptp-v4-0-008af03cd43b@pengutronix.de?part=5