[PATCH net 0/2] fixes two virtio-net related bugs.

[PATCH net 0/2] fixes two virtio-net related bugs.

[Xuan Zhuo]

As discussed in http://lore.kernel.org/all/20250919013450.111424-1-xuanzhuo@linux.alibaba.com
Commit #1 Move the flags into the existing if condition; the issue is that it introduces a
small amount of code duplication.

Commit #2 is new.

Xuan Zhuo (2):
  virtio-net: fix incorrect flags recording in big mode
  virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

 drivers/net/virtio_net.c   | 16 +++++++++++-----
 include/linux/virtio_net.h | 19 ++++++++++++-------
 2 files changed, 23 insertions(+), 12 deletions(-)

--
2.32.0.3.g01195cf9f

[PATCH net 1/2] virtio-net: fix incorrect flags recording in big mode

[Xuan Zhuo]

The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
checksummed packets handling") is to record the flags in advance, as
their value may be overwritten in the XDP case. However, the flags
recorded under big mode are incorrect, because in big mode, the passed
buf does not point to the rx buffer, but rather to the page of the
submitted buffer. This commit fixes this issue.

For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
buf address when unmapping for small packets") fixed it.

Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 975bdc5dab84..b4d1c09ff488 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2621,22 +2621,28 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 		return;
 	}
 
-	/* 1. Save the flags early, as the XDP program might overwrite them.
+	/* About the flags below:
+	 * 1. Save the flags early, as the XDP program might overwrite them.
 	 * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID
 	 * stay valid after XDP processing.
 	 * 2. XDP doesn't work with partially checksummed packets (refer to
 	 * virtnet_xdp_set()), so packets marked as
 	 * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing.
 	 */
-	flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
 
-	if (vi->mergeable_rx_bufs)
+	if (vi->mergeable_rx_bufs) {
+		flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
 		skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
 					stats);
-	else if (vi->big_packets)
+	} else if (vi->big_packets) {
+		void *p = page_address((struct page *)buf);
+
+		flags = ((struct virtio_net_common_hdr *)p)->hdr.flags;
 		skb = receive_big(dev, vi, rq, buf, len, stats);
-	else
+	} else {
+		flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags;
 		skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
+	}
 
 	if (unlikely(!skb))
 		return;
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net 1/2] virtio-net: fix incorrect flags recording in big mode

[Jason Wang]

On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> The purpose of commit 703eec1b2422 ("virtio_net: fixing XDP for fully
> checksummed packets handling") is to record the flags in advance, as
> their value may be overwritten in the XDP case. However, the flags
> recorded under big mode are incorrect, because in big mode, the passed
> buf does not point to the rx buffer, but rather to the page of the
> submitted buffer. This commit fixes this issue.
>
> For the small mode, the commit c11a49d58ad2 ("virtio_net: Fix mismatched
> buf address when unmapping for small packets") fixed it.
>
> Fixes: 703eec1b2422 ("virtio_net: fixing XDP for fully checksummed packets handling")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

[PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

The commit be50da3e9d4a ("net: virtio_net: implement exact header length
guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
feature in virtio-net.

This feature requires virtio-net to set hdr_len to the actual header
length of the packet when transmitting, the number of
bytes from the start of the packet to the beginning of the
transport-layer payload.

However, in practice, hdr_len was being set using skb_headlen(skb),
which is clearly incorrect. This commit fixes that issue.

Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 include/linux/virtio_net.h | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 20e0584db1dd..4273420a9ff9 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -217,20 +217,25 @@ 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);
+		u16 hdr_len;
 
-		/* This is a hint as to how much should be linear. */
-		hdr->hdr_len = __cpu_to_virtio16(little_endian,
-						 skb_headlen(skb));
+		hdr_len = skb_transport_offset(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)
+			hdr_len += tcp_hdrlen(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)
+			hdr_len += tcp_hdrlen(skb);
+		} else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
 			hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
-		else
+			hdr_len += sizeof(struct udphdr);
+		} else {
 			return -EINVAL;
+		}
+
+		hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
 		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
 			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
 	} else
-- 
2.32.0.3.g01195cf9f

Re: [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Jason Wang]

On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> The commit be50da3e9d4a ("net: virtio_net: implement exact header length
> guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
> feature in virtio-net.
>
> This feature requires virtio-net to set hdr_len to the actual header
> length of the packet when transmitting, the number of
> bytes from the start of the packet to the beginning of the
> transport-layer payload.
>
> However, in practice, hdr_len was being set using skb_headlen(skb),
> which is clearly incorrect. This commit fixes that issue.
>
> Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  include/linux/virtio_net.h | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 20e0584db1dd..4273420a9ff9 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -217,20 +217,25 @@ 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);
> +               u16 hdr_len;
>
> -               /* This is a hint as to how much should be linear. */
> -               hdr->hdr_len = __cpu_to_virtio16(little_endian,
> -                                                skb_headlen(skb));
> +               hdr_len = skb_transport_offset(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)
> +                       hdr_len += tcp_hdrlen(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)
> +                       hdr_len += tcp_hdrlen(skb);
> +               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
>                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> -               else

I think we need to deal with the GSO tunnel as well?

"""
    If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
    VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
    all the headers up to and including the inner transport.
"""

> +                       hdr_len += sizeof(struct udphdr);
> +               } else {
>                         return -EINVAL;
> +               }
> +
> +               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);

Should we at least check against the feature of VIRTIO_NET_F_GUEST_HDRLEN?

>                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
>                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>         } else
> --
> 2.32.0.3.g01195cf9f
>

Re: [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

On Fri, 26 Sep 2025 12:52:53 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > The commit be50da3e9d4a ("net: virtio_net: implement exact header length
> > guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
> > feature in virtio-net.
> >
> > This feature requires virtio-net to set hdr_len to the actual header
> > length of the packet when transmitting, the number of
> > bytes from the start of the packet to the beginning of the
> > transport-layer payload.
> >
> > However, in practice, hdr_len was being set using skb_headlen(skb),
> > which is clearly incorrect. This commit fixes that issue.
> >
> > Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  include/linux/virtio_net.h | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index 20e0584db1dd..4273420a9ff9 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -217,20 +217,25 @@ 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);
> > +               u16 hdr_len;
> >
> > -               /* This is a hint as to how much should be linear. */
> > -               hdr->hdr_len = __cpu_to_virtio16(little_endian,
> > -                                                skb_headlen(skb));
> > +               hdr_len = skb_transport_offset(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)
> > +                       hdr_len += tcp_hdrlen(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)
> > +                       hdr_len += tcp_hdrlen(skb);
> > +               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> >                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> > -               else
>
> I think we need to deal with the GSO tunnel as well?
>
> """
>     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
>     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
>     all the headers up to and including the inner transport.
> """

Sorry, I referred to the old spec. According to the latest spec, this case should indeed be considered.


>
> > +                       hdr_len += sizeof(struct udphdr);
> > +               } else {
> >                         return -EINVAL;
> > +               }
> > +
> > +               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
>
> Should we at least check against the feature of VIRTIO_NET_F_GUEST_HDRLEN?

I think we don't need to check for VIRTIO_NET_F_GUEST_HDRLEN, because regardless
of whether this feature has been negotiated or not, we always pass this value
hdr_len.

Previously, the value might have been incorrect but still acceptable; however,
once VIRTIO_NET_F_GUEST_HDRLEN is negotiated, incorrect values must no
longer be accepted. Therefore, we don't need to check VIRTIO_NET_F_GUEST_HDRLEN.
On the other hand, VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 should be checked.

Thanks.

>
> >                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> >                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> >         } else
> > --
> > 2.32.0.3.g01195cf9f
> >
>

Re: [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

On Fri, 26 Sep 2025 12:52:53 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > The commit be50da3e9d4a ("net: virtio_net: implement exact header length
> > guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
> > feature in virtio-net.
> >
> > This feature requires virtio-net to set hdr_len to the actual header
> > length of the packet when transmitting, the number of
> > bytes from the start of the packet to the beginning of the
> > transport-layer payload.
> >
> > However, in practice, hdr_len was being set using skb_headlen(skb),
> > which is clearly incorrect. This commit fixes that issue.
> >
> > Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >  include/linux/virtio_net.h | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > index 20e0584db1dd..4273420a9ff9 100644
> > --- a/include/linux/virtio_net.h
> > +++ b/include/linux/virtio_net.h
> > @@ -217,20 +217,25 @@ 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);
> > +               u16 hdr_len;
> >
> > -               /* This is a hint as to how much should be linear. */
> > -               hdr->hdr_len = __cpu_to_virtio16(little_endian,
> > -                                                skb_headlen(skb));
> > +               hdr_len = skb_transport_offset(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)
> > +                       hdr_len += tcp_hdrlen(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)
> > +                       hdr_len += tcp_hdrlen(skb);
> > +               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> >                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> > -               else
>
> I think we need to deal with the GSO tunnel as well?
>
> """
>     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
>     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
>     all the headers up to and including the inner transport.
> """

I checked the new code, VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
and VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 are not supported, so the next version
will not include these feature.

Thanks.


>
> > +                       hdr_len += sizeof(struct udphdr);
> > +               } else {
> >                         return -EINVAL;
> > +               }
> > +
> > +               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
>
> Should we at least check against the feature of VIRTIO_NET_F_GUEST_HDRLEN?
>
> >                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> >                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> >         } else
> > --
> > 2.32.0.3.g01195cf9f
> >
>

Re: [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Jason Wang]

On Thu, Oct 9, 2025 at 7:10 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Fri, 26 Sep 2025 12:52:53 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > The commit be50da3e9d4a ("net: virtio_net: implement exact header length
> > > guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
> > > feature in virtio-net.
> > >
> > > This feature requires virtio-net to set hdr_len to the actual header
> > > length of the packet when transmitting, the number of
> > > bytes from the start of the packet to the beginning of the
> > > transport-layer payload.
> > >
> > > However, in practice, hdr_len was being set using skb_headlen(skb),
> > > which is clearly incorrect. This commit fixes that issue.
> > >
> > > Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >  include/linux/virtio_net.h | 19 ++++++++++++-------
> > >  1 file changed, 12 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > index 20e0584db1dd..4273420a9ff9 100644
> > > --- a/include/linux/virtio_net.h
> > > +++ b/include/linux/virtio_net.h
> > > @@ -217,20 +217,25 @@ 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);
> > > +               u16 hdr_len;
> > >
> > > -               /* This is a hint as to how much should be linear. */
> > > -               hdr->hdr_len = __cpu_to_virtio16(little_endian,
> > > -                                                skb_headlen(skb));
> > > +               hdr_len = skb_transport_offset(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)
> > > +                       hdr_len += tcp_hdrlen(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)
> > > +                       hdr_len += tcp_hdrlen(skb);
> > > +               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> > >                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> > > -               else
> >
> > I think we need to deal with the GSO tunnel as well?
> >
> > """
> >     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
> >     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
> >     all the headers up to and including the inner transport.
> > """
>
> I checked the new code, VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
> and VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 are not supported, so the next version
> will not include these feature.
>
> Thanks.

I may miss something but we had:

        if (skb->protocol == htons(ETH_P_IPV6))
                hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6;
        else
                hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4;

in virtio_net_hdr_tnl_from_skb() now.

Thanks

>
>
> >
> > > +                       hdr_len += sizeof(struct udphdr);
> > > +               } else {
> > >                         return -EINVAL;
> > > +               }
> > > +
> > > +               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> >
> > Should we at least check against the feature of VIRTIO_NET_F_GUEST_HDRLEN?
> >
> > >                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> > >                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> > >         } else
> > > --
> > > 2.32.0.3.g01195cf9f
> > >
> >
>

Re: [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

[Xuan Zhuo]

On Fri, 10 Oct 2025 08:36:33 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Thu, Oct 9, 2025 at 7:10 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Fri, 26 Sep 2025 12:52:53 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Thu, Sep 25, 2025 at 10:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > The commit be50da3e9d4a ("net: virtio_net: implement exact header length
> > > > guest feature") introduces support for the VIRTIO_NET_F_GUEST_HDRLEN
> > > > feature in virtio-net.
> > > >
> > > > This feature requires virtio-net to set hdr_len to the actual header
> > > > length of the packet when transmitting, the number of
> > > > bytes from the start of the packet to the beginning of the
> > > > transport-layer payload.
> > > >
> > > > However, in practice, hdr_len was being set using skb_headlen(skb),
> > > > which is clearly incorrect. This commit fixes that issue.
> > > >
> > > > Fixes: be50da3e9d4a ("net: virtio_net: implement exact header length guest feature")
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > ---
> > > >  include/linux/virtio_net.h | 19 ++++++++++++-------
> > > >  1 file changed, 12 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> > > > index 20e0584db1dd..4273420a9ff9 100644
> > > > --- a/include/linux/virtio_net.h
> > > > +++ b/include/linux/virtio_net.h
> > > > @@ -217,20 +217,25 @@ 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);
> > > > +               u16 hdr_len;
> > > >
> > > > -               /* This is a hint as to how much should be linear. */
> > > > -               hdr->hdr_len = __cpu_to_virtio16(little_endian,
> > > > -                                                skb_headlen(skb));
> > > > +               hdr_len = skb_transport_offset(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)
> > > > +                       hdr_len += tcp_hdrlen(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)
> > > > +                       hdr_len += tcp_hdrlen(skb);
> > > > +               } else if (sinfo->gso_type & SKB_GSO_UDP_L4) {
> > > >                         hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
> > > > -               else
> > >
> > > I think we need to deal with the GSO tunnel as well?
> > >
> > > """
> > >     If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
> > >     VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
> > >     all the headers up to and including the inner transport.
> > > """
> >
> > I checked the new code, VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
> > and VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 are not supported, so the next version
> > will not include these feature.
> >
> > Thanks.
>
> I may miss something but we had:
>
>         if (skb->protocol == htons(ETH_P_IPV6))
>                 hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6;
>         else
>                 hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4;
>
> in virtio_net_hdr_tnl_from_skb() now.

I checked the *.c files only, so I missed it.

And I found that skb_headroom is used here, that should be a error.


	if (skb->protocol == htons(ETH_P_IPV6))
		hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6;
	else
		hdr->gso_type |= VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4;

	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM)
		hdr->flags |= VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM;

	inner_nh = skb->inner_network_header - skb_headroom(skb);
	outer_th = skb->transport_header - skb_headroom(skb);

Thanks

>
> Thanks
>
> >
> >
> > >
> > > > +                       hdr_len += sizeof(struct udphdr);
> > > > +               } else {
> > > >                         return -EINVAL;
> > > > +               }
> > > > +
> > > > +               hdr->hdr_len = __cpu_to_virtio16(little_endian, hdr_len);
> > >
> > > Should we at least check against the feature of VIRTIO_NET_F_GUEST_HDRLEN?
> > >
> > > >                 if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> > > >                         hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> > > >         } else
> > > > --
> > > > 2.32.0.3.g01195cf9f
> > > >
> > >
> >
>

[syzbot ci] Re: fixes two virtio-net related bugs.

[syzbot ci]

syzbot ci has tested the following series

[v1] fixes two virtio-net related bugs.
https://lore.kernel.org/all/20250925022537.91774-1-xuanzhuo@linux.alibaba.com
* [PATCH net 1/2] virtio-net: fix incorrect flags recording in big mode
* [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN

and found the following issue:
WARNING in virtio_net_hdr_from_skb

Full report is available here:
https://ci.syzbot.org/series/41a78b3d-b982-4507-b02f-1991c8d827c9

***

WARNING in virtio_net_hdr_from_skb

tree:      net
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
base:      f8b4687151021db61841af983f1cb7be6915d4ef
arch:      amd64
compiler:  Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
config:    https://ci.syzbot.org/builds/bb3f28da-9370-44d7-be20-9c443d5ebfc9/config
C repro:   https://ci.syzbot.org/findings/7af48589-24a5-4e09-bfe0-1969f5d98f4d/c_repro
syz repro: https://ci.syzbot.org/findings/7af48589-24a5-4e09-bfe0-1969f5d98f4d/syz_repro

syz.0.17 uses obsolete (PF_INET,SOCK_PACKET)
------------[ cut here ]------------
WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 skb_transport_header include/linux/skbuff.h:3024 [inline]
WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 skb_transport_offset include/linux/skbuff.h:3175 [inline]
WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 virtio_net_hdr_from_skb+0x669/0xa30 include/linux/virtio_net.h:222
Modules linked in:
CPU: 0 UID: 0 PID: 6010 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:skb_transport_header include/linux/skbuff.h:3024 [inline]
RIP: 0010:skb_transport_offset include/linux/skbuff.h:3175 [inline]
RIP: 0010:virtio_net_hdr_from_skb+0x669/0xa30 include/linux/virtio_net.h:222
Code: 00 00 00 fc ff df 0f b6 04 10 84 c0 0f 85 b0 03 00 00 41 c6 45 00 05 48 8b 74 24 08 83 c6 08 e9 ca fd ff ff e8 c8 1c 72 f7 90 <0f> 0b 90 e9 a7 fa ff ff e8 ba 1c 72 f7 90 0f 0b 90 e9 62 fc ff ff
RSP: 0018:ffffc90002def490 EFLAGS: 00010293
RAX: ffffffff8a4da238 RBX: 1ffff11004dafca6 RCX: ffff888023c9b980
RDX: 0000000000000000 RSI: 000000000000ffff RDI: 000000000000ffff
RBP: 1ffff1100537cbd8 R08: ffff88804138e059 R09: 0000000000000000
R10: ffff88804138e050 R11: ffffed1008271c0c R12: ffff888029be4000
R13: ffff888026d7e536 R14: ffff888029be5ec4 R15: 000000000000ffff
FS:  0000555557e59500(0000) GS:ffff8880b8615000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000200000003000 CR3: 00000000414a0000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 tpacket_rcv+0x15c8/0x3290 net/packet/af_packet.c:2416
 __netif_receive_skb_one_core net/core/dev.c:5991 [inline]
 __netif_receive_skb+0x164/0x380 net/core/dev.c:6104
 netif_receive_skb_internal net/core/dev.c:6190 [inline]
 netif_receive_skb+0x1cb/0x790 net/core/dev.c:6249
 tun_rx_batched+0x1b9/0x730 drivers/net/tun.c:1485
 tun_get_user+0x2aa2/0x3e20 drivers/net/tun.c:1950
 tun_chr_write_iter+0x113/0x200 drivers/net/tun.c:1996
 new_sync_write fs/read_write.c:593 [inline]
 vfs_write+0x5c9/0xb30 fs/read_write.c:686
 ksys_write+0x145/0x250 fs/read_write.c:738
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe74f78ec29
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fffd7d9aa28 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007fe74f9d5fa0 RCX: 00007fe74f78ec29
RDX: 0000000000000fb5 RSI: 0000200000002780 RDI: 0000000000000004
RBP: 00007fe74f811e41 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fe74f9d5fa0 R14: 00007fe74f9d5fa0 R15: 0000000000000003
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

Re: [syzbot ci] Re: fixes two virtio-net related bugs.

[Marco Elver]

#syz upstream

On Thu, 25 Sept 2025 at 09:13, syzbot ci
<syzbot+ci4290219e4732157d@syzkaller.appspotmail.com> wrote:
>
> syzbot ci has tested the following series
>
> [v1] fixes two virtio-net related bugs.
> https://lore.kernel.org/all/20250925022537.91774-1-xuanzhuo@linux.alibaba.com
> * [PATCH net 1/2] virtio-net: fix incorrect flags recording in big mode
> * [PATCH net 2/2] virtio-net: correct hdr_len handling for VIRTIO_NET_F_GUEST_HDRLEN
>
> and found the following issue:
> WARNING in virtio_net_hdr_from_skb
>
> Full report is available here:
> https://ci.syzbot.org/series/41a78b3d-b982-4507-b02f-1991c8d827c9
>
> ***
>
> WARNING in virtio_net_hdr_from_skb
>
> tree:      net
> URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
> base:      f8b4687151021db61841af983f1cb7be6915d4ef
> arch:      amd64
> compiler:  Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> config:    https://ci.syzbot.org/builds/bb3f28da-9370-44d7-be20-9c443d5ebfc9/config
> C repro:   https://ci.syzbot.org/findings/7af48589-24a5-4e09-bfe0-1969f5d98f4d/c_repro
> syz repro: https://ci.syzbot.org/findings/7af48589-24a5-4e09-bfe0-1969f5d98f4d/syz_repro
>
> syz.0.17 uses obsolete (PF_INET,SOCK_PACKET)
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 skb_transport_header include/linux/skbuff.h:3024 [inline]
> WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 skb_transport_offset include/linux/skbuff.h:3175 [inline]
> WARNING: CPU: 0 PID: 6010 at ./include/linux/skbuff.h:3024 virtio_net_hdr_from_skb+0x669/0xa30 include/linux/virtio_net.h:222
> Modules linked in:
> CPU: 0 UID: 0 PID: 6010 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> RIP: 0010:skb_transport_header include/linux/skbuff.h:3024 [inline]
> RIP: 0010:skb_transport_offset include/linux/skbuff.h:3175 [inline]
> RIP: 0010:virtio_net_hdr_from_skb+0x669/0xa30 include/linux/virtio_net.h:222
> Code: 00 00 00 fc ff df 0f b6 04 10 84 c0 0f 85 b0 03 00 00 41 c6 45 00 05 48 8b 74 24 08 83 c6 08 e9 ca fd ff ff e8 c8 1c 72 f7 90 <0f> 0b 90 e9 a7 fa ff ff e8 ba 1c 72 f7 90 0f 0b 90 e9 62 fc ff ff
> RSP: 0018:ffffc90002def490 EFLAGS: 00010293
> RAX: ffffffff8a4da238 RBX: 1ffff11004dafca6 RCX: ffff888023c9b980
> RDX: 0000000000000000 RSI: 000000000000ffff RDI: 000000000000ffff
> RBP: 1ffff1100537cbd8 R08: ffff88804138e059 R09: 0000000000000000
> R10: ffff88804138e050 R11: ffffed1008271c0c R12: ffff888029be4000
> R13: ffff888026d7e536 R14: ffff888029be5ec4 R15: 000000000000ffff
> FS:  0000555557e59500(0000) GS:ffff8880b8615000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000200000003000 CR3: 00000000414a0000 CR4: 00000000000006f0
> Call Trace:
>  <TASK>
>  tpacket_rcv+0x15c8/0x3290 net/packet/af_packet.c:2416
>  __netif_receive_skb_one_core net/core/dev.c:5991 [inline]
>  __netif_receive_skb+0x164/0x380 net/core/dev.c:6104
>  netif_receive_skb_internal net/core/dev.c:6190 [inline]
>  netif_receive_skb+0x1cb/0x790 net/core/dev.c:6249
>  tun_rx_batched+0x1b9/0x730 drivers/net/tun.c:1485
>  tun_get_user+0x2aa2/0x3e20 drivers/net/tun.c:1950
>  tun_chr_write_iter+0x113/0x200 drivers/net/tun.c:1996
>  new_sync_write fs/read_write.c:593 [inline]
>  vfs_write+0x5c9/0xb30 fs/read_write.c:686
>  ksys_write+0x145/0x250 fs/read_write.c:738
>  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>  do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fe74f78ec29
> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fffd7d9aa28 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00007fe74f9d5fa0 RCX: 00007fe74f78ec29
> RDX: 0000000000000fb5 RSI: 0000200000002780 RDI: 0000000000000004
> RBP: 00007fe74f811e41 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 00007fe74f9d5fa0 R14: 00007fe74f9d5fa0 R15: 0000000000000003
>  </TASK>
>
>
> ***
>
> If these findings have caused you to resend the series or submit a
> separate fix, please add the following tag to your commit message:
>   Tested-by: syzbot@syzkaller.appspotmail.com
>
> ---
> This report is generated by a bot. It may contain errors.
> syzbot ci engineers can be reached at syzkaller@googlegroups.com.
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/syzkaller-bugs/68d4eb92.050a0220.25d7ab.0003.GAE%40google.com.

Re: Re: [syzbot ci] Re: fixes two virtio-net related bugs.

[syzbot ci]

Failed to process the command. Contact syzkaller@googlegroups.com.