Re: [PATCH net v2] net: drop bad gso csum_start and offset in virtio_net_hdr
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2024-09-09 15:20:11
Also in:
stable
quoted
quoted
quoted
So I guess VIRTIO_NET_HDR_GSO_* without VIRTIO_NET_HDR_F_DATA_VALID would be wrong on rx. But the new check if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { [...] case SKB_GSO_TCPV4: case SKB_GSO_TCPV6: if (skb->csum_offset != offsetof(struct tcphdr, check)) return -EINVAL; should be limited to callers of virtio_net_hdr_to_skb on the tx/GSO path. Looking what the cleanest/minimal patch is to accomplish that.virtio_net_hdr_to_skb() translates virtio-net header to skb metadata, so it's RX. For TX the helper should be virtio_net_hdr_from_skb() which translates skb metadata to virtio hdr.virtio_net_hdr_to_skb is used by PF_PACKET, tun and tapExactly.quoted
when injecting a packet into the egress path.For tuntap it's still the RX path. For PF_PACEKT and macvtap, it's the tx. Maybe a new parameter to virtio_net_hdr_to_skb()?
This is the most straightforward approach. But requires changse to all callers, in a patch targeting all the stable branches. I'd prefer if we can detect ingress vs egress directly. Based on ip_summed, pkt_type, is_skb_wmem or so. But so far have not found a suitable condition. I noticed something else: as you point out TUN is ingress. Unlike virtnet_receive, it does not set ip_summed to CHECKSUM_UNNECESSARY if VIRTIO_NET_HDR_F_DATA_VALID. It probably should. GRO expects packets to have had their integrity verified. CHECKSUM_NONE on ingress is not correct for GRO. And also related: no GRO should be generated by a device unless VIRTIO_NET_HDR_F_DATA_VALID is also passed? I have to check the spec if it says anything about this.