The virtio spec says:
    If one of the VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options have
    been negotiated:
        If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
        and gso_type differs from VIRTIO_NET_HDR_GSO_NONE, the driver
        MUST set hdr_len to a value equal to the length of the headers,
        including the transport header.

        If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
        or gso_type is VIRTIO_NET_HDR_GSO_NONE, the driver SHOULD set
        hdr_len to a value not less than the length of the headers,
        including the transport header.

So if the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated, the
hdr->hdr_len should be eth header + ip header + tcp/udp header.

But now:
    hdr->hdr_len = __cpu_to_virtio15(little_endian, skb_headlen(skb));

The skb_headlen() returns the linear space of the skb, not the header
size that only match the case the VIRTIO_NET_F_GUEST_HDRLEN feature has
not been negotiated, or gso_type is VIRTIO_NET_HDR_GSO_NONE.

We do not check the feature of the device. This function is a common
function used by many places. So we do more stricter work whatever
the features is negotiated.

For the case skb_is_gso(skb) is false, if none of the
VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options have been negotiated,
the spec not define the action of setting hdr_len. Here I set it to
skb_headlen(). If one of the above features have been negotiated, we
should set a value not less than the length of "eth header + ip header +
tcp/udp header". So the skb_headlen() also is a valid value.

For the case skb_is_gso(skb) is true, it implies that one of
VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options MUST have been
negotiated. If the VIRTIO_NET_F_GUEST_HDRLEN is negotiated, we MUST set
it to the length of "eth header + ip header + tcp/udp header".
If the VIRTIO_NET_F_GUEST_HDRLEN is not negotiated, that still be a
valid value.

Signed-off-by: Xuan Zhuo <[email protected]>
Reported-by: Spike Du <[email protected]>
---
 include/linux/virtio_net.h | 41 ++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 4dfa9b69ca8d..51d93f9762d7 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -201,24 +201,45 @@ static inline int virtio_net_hdr_from_skb(const struct 
sk_buff *skb,
 
        if (skb_is_gso(skb)) {
                struct skb_shared_info *sinfo = skb_shinfo(skb);
+               u32 hdrlen;
 
-               /* This is a hint as to how much should be linear. */
-               hdr->hdr_len = __cpu_to_virtio16(little_endian,
-                                                skb_headlen(skb));
-               hdr->gso_size = __cpu_to_virtio16(little_endian,
-                                                 sinfo->gso_size);
-               if (sinfo->gso_type & SKB_GSO_TCPV4)
+               if (sinfo->gso_type & SKB_GSO_TCPV4) {
                        hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
-               else if (sinfo->gso_type & SKB_GSO_TCPV6)
+                       hdrlen = tcp_hdrlen(skb) + skb_transport_offset(skb);
+
+               } else if (sinfo->gso_type & SKB_GSO_TCPV6) {
                        hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
-               else if (sinfo->gso_type & SKB_GSO_UDP_L4)
+                       hdrlen = tcp_hdrlen(skb) + skb_transport_offset(skb);
+
+               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
                        hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
-               else
+                       hdrlen = sizeof(struct udphdr) + 
skb_transport_offset(skb);
+
+               } else {
                        return -EINVAL;
+               }
+
+               /* One of VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options MUST
+                * have been negotiated. If the VIRTIO_NET_F_GUEST_HDRLEN is
+                * negotiated, we MUST set it to the length of "eth header + ip
+                * header + tcp/udp header".  If the VIRTIO_NET_F_GUEST_HDRLEN
+                * is not negotiated, that still be a valid value.
+                */
+               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdrlen);
+               hdr->gso_size = __cpu_to_virtio16(little_endian,
+                                                 sinfo->gso_size);
+
                if (sinfo->gso_type & SKB_GSO_TCP_ECN)
                        hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
-       } else
+       } else {
+               /* If one of the VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO
+                * options have been negotiated, we should set hdr_len to a
+                * value not less than the length of "eth header + ip header +
+                * tcp/udp header".
+                */
+               hdr->hdr_len = __cpu_to_virtio16(little_endian, 
skb_headlen(skb));
                hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+       }
 
        if (skb->ip_summed == CHECKSUM_PARTIAL) {
                hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-- 
2.32.0.3.g01195cf9f


Reply via email to