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 <[email protected]> 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-008af03cd...@pengutronix.de?part=5
