[PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Wang Liang]

When sending a packet with virtio_net_hdr to tun device, if the gso_type
in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
size, below crash may happen.

  ------------[ cut here ]------------
  kernel BUG at net/core/skbuff.c:4572!
  Oops: invalid opcode: 0000 [#1] SMP NOPTI
  CPU: 0 UID: 0 PID: 62 Comm: mytest Not tainted 6.16.0-rc7 #203 PREEMPT(voluntary)
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
  RIP: 0010:skb_pull_rcsum+0x8e/0xa0
  Code: 00 00 5b c3 cc cc cc cc 8b 93 88 00 00 00 f7 da e8 37 44 38 00 f7 d8 89 83 88 00 00 00 48 8b 83 c8 00 00 00 5b c3 cc cc cc cc <0f> 0b 0f 0b 66 66 2e 0f 1f 84 00 000
  RSP: 0018:ffffc900001fba38 EFLAGS: 00000297
  RAX: 0000000000000004 RBX: ffff8880040c1000 RCX: ffffc900001fb948
  RDX: ffff888003e6d700 RSI: 0000000000000008 RDI: ffff88800411a062
  RBP: ffff8880040c1000 R08: 0000000000000000 R09: 0000000000000001
  R10: ffff888003606c00 R11: 0000000000000001 R12: 0000000000000000
  R13: ffff888004060900 R14: ffff888004050000 R15: ffff888004060900
  FS:  000000002406d3c0(0000) GS:ffff888084a19000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000020000040 CR3: 0000000004007000 CR4: 00000000000006f0
  Call Trace:
   <TASK>
   udp_queue_rcv_one_skb+0x176/0x4b0 net/ipv4/udp.c:2445
   udp_queue_rcv_skb+0x155/0x1f0 net/ipv4/udp.c:2475
   udp_unicast_rcv_skb+0x71/0x90 net/ipv4/udp.c:2626
   __udp4_lib_rcv+0x433/0xb00 net/ipv4/udp.c:2690
   ip_protocol_deliver_rcu+0xa6/0x160 net/ipv4/ip_input.c:205
   ip_local_deliver_finish+0x72/0x90 net/ipv4/ip_input.c:233
   ip_sublist_rcv_finish+0x5f/0x70 net/ipv4/ip_input.c:579
   ip_sublist_rcv+0x122/0x1b0 net/ipv4/ip_input.c:636
   ip_list_rcv+0xf7/0x130 net/ipv4/ip_input.c:670
   __netif_receive_skb_list_core+0x21d/0x240 net/core/dev.c:6067
   netif_receive_skb_list_internal+0x186/0x2b0 net/core/dev.c:6210
   napi_complete_done+0x78/0x180 net/core/dev.c:6580
   tun_get_user+0xa63/0x1120 drivers/net/tun.c:1909
   tun_chr_write_iter+0x65/0xb0 drivers/net/tun.c:1984
   vfs_write+0x300/0x420 fs/read_write.c:593
   ksys_write+0x60/0xd0 fs/read_write.c:686
   do_syscall_64+0x50/0x1c0 arch/x86/entry/syscall_64.c:63
   </TASK>

To trigger gso segment in udp_queue_rcv_skb(), we should also set option
UDP_ENCAP_ESPINUDP to enable udp_sk(sk)->encap_rcv. When the encap_rcv
hook return 1 in udp_queue_rcv_one_skb(), udp_csum_pull_header() will try
to pull udphdr, but the skb size has been segmented to gso size, which
leads to this crash.

Only udp has this probloem. Add check for the minimum value of gso size in
virtio_net_hdr_to_skb().

Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
Fixes: 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing in a tunnel")
Signed-off-by: Wang Liang <wangliang74@huawei.com>
---
 include/linux/virtio_net.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 02a9f4dc594d..0533101642bd 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -157,11 +157,13 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 		u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
 		unsigned int nh_off = p_off;
 		struct skb_shared_info *shinfo = skb_shinfo(skb);
+		u16 min_gso_size = 0;
 
 		switch (gso_type & ~SKB_GSO_TCP_ECN) {
 		case SKB_GSO_UDP:
 			/* UFO may not include transport header in gso_size. */
 			nh_off -= thlen;
+			min_gso_size = sizeof(struct udphdr) + 1;
 			break;
 		case SKB_GSO_UDP_L4:
 			if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
@@ -172,6 +174,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 				return -EINVAL;
 			if (gso_type != SKB_GSO_UDP_L4)
 				return -EINVAL;
+			min_gso_size = sizeof(struct udphdr) + 1;
 			break;
 		case SKB_GSO_TCPV4:
 		case SKB_GSO_TCPV6:
@@ -182,7 +185,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 		}
 
 		/* Kernel has a special handling for GSO_BY_FRAGS. */
-		if (gso_size == GSO_BY_FRAGS)
+		if ((gso_size == GSO_BY_FRAGS) || (gso_size < min_gso_size))
 			return -EINVAL;
 
 		/* Too small packets are not really GSO ones. */
-- 
2.34.1

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Willem de Bruijn]

Wang Liang wrote:
> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> size, below crash may happen.
> 
>   ------------[ cut here ]------------
>   kernel BUG at net/core/skbuff.c:4572!

The BUG_ON hit:

	void *skb_pull_rcsum(struct sk_buff *skb, unsigned int len)
	{
		unsigned char *data = skb->data;

		BUG_ON(len > skb->len);

>   Oops: invalid opcode: 0000 [#1] SMP NOPTI
>   CPU: 0 UID: 0 PID: 62 Comm: mytest Not tainted 6.16.0-rc7 #203 PREEMPT(voluntary)
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
>   RIP: 0010:skb_pull_rcsum+0x8e/0xa0

From udp_csum_pull_header

>   Code: 00 00 5b c3 cc cc cc cc 8b 93 88 00 00 00 f7 da e8 37 44 38 00 f7 d8 89 83 88 00 00 00 48 8b 83 c8 00 00 00 5b c3 cc cc cc cc <0f> 0b 0f 0b 66 66 2e 0f 1f 84 00 000
>   RSP: 0018:ffffc900001fba38 EFLAGS: 00000297
>   RAX: 0000000000000004 RBX: ffff8880040c1000 RCX: ffffc900001fb948
>   RDX: ffff888003e6d700 RSI: 0000000000000008 RDI: ffff88800411a062
>   RBP: ffff8880040c1000 R08: 0000000000000000 R09: 0000000000000001
>   R10: ffff888003606c00 R11: 0000000000000001 R12: 0000000000000000
>   R13: ffff888004060900 R14: ffff888004050000 R15: ffff888004060900
>   FS:  000000002406d3c0(0000) GS:ffff888084a19000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 0000000020000040 CR3: 0000000004007000 CR4: 00000000000006f0
>   Call Trace:
>    <TASK>
>    udp_queue_rcv_one_skb+0x176/0x4b0 net/ipv4/udp.c:2445
>    udp_queue_rcv_skb+0x155/0x1f0 net/ipv4/udp.c:2475
>    udp_unicast_rcv_skb+0x71/0x90 net/ipv4/udp.c:2626
>    __udp4_lib_rcv+0x433/0xb00 net/ipv4/udp.c:2690
>    ip_protocol_deliver_rcu+0xa6/0x160 net/ipv4/ip_input.c:205
>    ip_local_deliver_finish+0x72/0x90 net/ipv4/ip_input.c:233
>    ip_sublist_rcv_finish+0x5f/0x70 net/ipv4/ip_input.c:579
>    ip_sublist_rcv+0x122/0x1b0 net/ipv4/ip_input.c:636
>    ip_list_rcv+0xf7/0x130 net/ipv4/ip_input.c:670
>    __netif_receive_skb_list_core+0x21d/0x240 net/core/dev.c:6067
>    netif_receive_skb_list_internal+0x186/0x2b0 net/core/dev.c:6210
>    napi_complete_done+0x78/0x180 net/core/dev.c:6580
>    tun_get_user+0xa63/0x1120 drivers/net/tun.c:1909
>    tun_chr_write_iter+0x65/0xb0 drivers/net/tun.c:1984
>    vfs_write+0x300/0x420 fs/read_write.c:593
>    ksys_write+0x60/0xd0 fs/read_write.c:686
>    do_syscall_64+0x50/0x1c0 arch/x86/entry/syscall_64.c:63
>    </TASK>
> 
> To trigger gso segment in udp_queue_rcv_skb(), we should also set option
> UDP_ENCAP_ESPINUDP to enable udp_sk(sk)->encap_rcv. When the encap_rcv
> hook return 1 in udp_queue_rcv_one_skb(), udp_csum_pull_header() will try
> to pull udphdr, but the skb size has been segmented to gso size, which
> leads to this crash.
> 
> Only udp has this probloem. Add check for the minimum value of gso size in
> virtio_net_hdr_to_skb().
> 
> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
> Fixes: 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing in a tunnel")
> Signed-off-by: Wang Liang <wangliang74@huawei.com>
> ---
>  include/linux/virtio_net.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 02a9f4dc594d..0533101642bd 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -157,11 +157,13 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
>  		unsigned int nh_off = p_off;
>  		struct skb_shared_info *shinfo = skb_shinfo(skb);
> +		u16 min_gso_size = 0;
>  
>  		switch (gso_type & ~SKB_GSO_TCP_ECN) {
>  		case SKB_GSO_UDP:
>  			/* UFO may not include transport header in gso_size. */
>  			nh_off -= thlen;
> +			min_gso_size = sizeof(struct udphdr) + 1;
>  			break;
>  		case SKB_GSO_UDP_L4:
>  			if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> @@ -172,6 +174,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  				return -EINVAL;
>  			if (gso_type != SKB_GSO_UDP_L4)
>  				return -EINVAL;
> +			min_gso_size = sizeof(struct udphdr) + 1;
>  			break;
>  		case SKB_GSO_TCPV4:
>  		case SKB_GSO_TCPV6:
> @@ -182,7 +185,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		}
>  
>  		/* Kernel has a special handling for GSO_BY_FRAGS. */
> -		if (gso_size == GSO_BY_FRAGS)
> +		if ((gso_size == GSO_BY_FRAGS) || (gso_size < min_gso_size))

gso_size is the size of the segment payload, excluding the transport
header.

This is probably not the right approach.

Not sure how a GSO skb can be built that is shorter than even the
transport header. Maybe an skb_dump of the GSO skb can be elucidating.
>  			return -EINVAL;
>  
>  		/* Too small packets are not really GSO ones. */
> -- 
> 2.34.1
> 

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Wang Liang]

在 2025/7/24 21:29, Willem de Bruijn 写道:
> Wang Liang wrote:
>> When sending a packet with virtio_net_hdr to tun device, if the gso_type
>> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
>> size, below crash may happen.
>>
> gso_size is the size of the segment payload, excluding the transport
> header.
>
> This is probably not the right approach.
>
> Not sure how a GSO skb can be built that is shorter than even the
> transport header. Maybe an skb_dump of the GSO skb can be elucidating.
>>   			return -EINVAL;
>>   
>>   		/* Too small packets are not really GSO ones. */
>> -- 
>> 2.34.1
>>

Thanks for your review!

Here is the skb_dump result:

     skb len=4 headroom=98 headlen=4 tailroom=282
     mac=(64,14) mac_len=14 net=(78,20) trans=98
     shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
     csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
     hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=4
     priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
     encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
     dev name=tun0 feat=0x00002000000048c1
     skb headroom: 00000000: 20 00 00 00 10 00 05 00 c4 2c 83 68 00 00 00 00
     skb headroom: 00000010: 00 00 00 00 04 00 00 00 01 00 00 00 01 00 00 00
     skb headroom: 00000020: 08 00 1d 00 09 00 00 00 09 00 03 00 74 75 6e 30
     skb headroom: 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb headroom: 00000040: 01 80 c2 00 00 00 ff ff ff ff ff ff 08 00 45 00
     skb headroom: 00000050: 00 18 00 00 20 00 00 11 ba d4 00 00 00 00 e0 00
     skb headroom: 00000060: 00 01
     skb linear:   00000000: 00 9c d0 90
     skb tailroom: 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 000000f0: 00 00 00 00 00 00 00 00 00 00 00 0b 0e 04 80 88
     skb tailroom: 00000100: ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     skb tailroom: 00000110: 00 00 00 00 00 00 00 00 00 00

The following C code can reproduce this issue:

     #include <string.h>
     #include <fcntl.h>
     #include <sys/ioctl.h>
     #include <linux/if.h>
     #include <linux/if_tun.h>
     #include <linux/virtio_net.h>
     #include <netinet/ip.h>
     #include <netinet/udp.h>

     int main(void)
     {
         // create udp socket, set option UDP_ENCAP_ESPINUDP
         int udp_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
         struct sockaddr_in server_addr = {
             .sin_family = AF_INET,
             .sin_port = htons(20004),
             .sin_addr.s_addr = inet_addr("224.0.0.1"),
         };
         bind(udp_fd, (struct sockaddr *)&server_addr, sizeof(server_addr));
         int val = UDP_ENCAP_ESPINUDP;
         setsockopt(udp_fd, IPPROTO_UDP, UDP_ENCAP, &val, sizeof(val));

         // send udp packet to tun/tap dev
         system("ip tuntap add dev tun0 mode tap");
         system("ip link set dev tun0 up");
         int fd = open("/dev/net/tun", O_RDWR);
         struct ifreq ifr = {
             .ifr_flags = IFF_TAP | IFF_NAPI | IFF_NAPI_FRAGS | 
IFF_ONE_QUEUE | IFF_VNET_HDR,
             .ifr_name  = "tun0",
         };
         ioctl(fd, TUNSETIFF, &ifr);
         struct tun_pi pi = { 0 };
         struct virtio_net_hdr gso = {
             .flags = VIRTIO_NET_HDR_F_NEEDS_CSUM,
             .gso_type = VIRTIO_NET_HDR_GSO_UDP,
             .hdr_len = 64,
             .gso_size = 4,
             .csum_start = 76,
             .csum_offset = 0,
         };
         struct ethhdr eth = {
             .h_dest   = {0x01, 0x80, 0xc2, 0x00, 0x00, 0x00},
             .h_source = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
             .h_proto  = htons(0x0800),
         };
         struct iphdr iph = {
             .version = 4,
             .ihl = 5,
             .tot_len = htons(176),
             .protocol = IPPROTO_UDP,
             .saddr = 0,
             .daddr = inet_addr("224.0.0.1"),
             .check = 0x3cda,
         };
         struct udphdr uh = {
             .source = 0,
             .dest = htons(20004),
             .len = htons(156),
             .check = 0x2363,
         };
         char buf[204] = { 0 };
         memcpy(buf, &pi, 4);
         memcpy(buf + 4, &gso, 10);
         memcpy(buf + 14, &eth, 14);
         memcpy(buf + 28, &iph, 20);
         memcpy(buf + 48, &uh, 8);
         write(fd, buf, sizeof(buf));
         close(fd);
         return 0;
     }

------
Best regards
Wang Liang

>

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Xuan Zhuo]

On Thu, 24 Jul 2025 16:30:05 +0800, Wang Liang <wangliang74@huawei.com> wrote:
> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> size, below crash may happen.
>
>   ------------[ cut here ]------------
>   kernel BUG at net/core/skbuff.c:4572!
>   Oops: invalid opcode: 0000 [#1] SMP NOPTI
>   CPU: 0 UID: 0 PID: 62 Comm: mytest Not tainted 6.16.0-rc7 #203 PREEMPT(voluntary)
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
>   RIP: 0010:skb_pull_rcsum+0x8e/0xa0
>   Code: 00 00 5b c3 cc cc cc cc 8b 93 88 00 00 00 f7 da e8 37 44 38 00 f7 d8 89 83 88 00 00 00 48 8b 83 c8 00 00 00 5b c3 cc cc cc cc <0f> 0b 0f 0b 66 66 2e 0f 1f 84 00 000
>   RSP: 0018:ffffc900001fba38 EFLAGS: 00000297
>   RAX: 0000000000000004 RBX: ffff8880040c1000 RCX: ffffc900001fb948
>   RDX: ffff888003e6d700 RSI: 0000000000000008 RDI: ffff88800411a062
>   RBP: ffff8880040c1000 R08: 0000000000000000 R09: 0000000000000001
>   R10: ffff888003606c00 R11: 0000000000000001 R12: 0000000000000000
>   R13: ffff888004060900 R14: ffff888004050000 R15: ffff888004060900
>   FS:  000000002406d3c0(0000) GS:ffff888084a19000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 0000000020000040 CR3: 0000000004007000 CR4: 00000000000006f0
>   Call Trace:
>    <TASK>
>    udp_queue_rcv_one_skb+0x176/0x4b0 net/ipv4/udp.c:2445
>    udp_queue_rcv_skb+0x155/0x1f0 net/ipv4/udp.c:2475
>    udp_unicast_rcv_skb+0x71/0x90 net/ipv4/udp.c:2626
>    __udp4_lib_rcv+0x433/0xb00 net/ipv4/udp.c:2690
>    ip_protocol_deliver_rcu+0xa6/0x160 net/ipv4/ip_input.c:205
>    ip_local_deliver_finish+0x72/0x90 net/ipv4/ip_input.c:233
>    ip_sublist_rcv_finish+0x5f/0x70 net/ipv4/ip_input.c:579
>    ip_sublist_rcv+0x122/0x1b0 net/ipv4/ip_input.c:636
>    ip_list_rcv+0xf7/0x130 net/ipv4/ip_input.c:670
>    __netif_receive_skb_list_core+0x21d/0x240 net/core/dev.c:6067
>    netif_receive_skb_list_internal+0x186/0x2b0 net/core/dev.c:6210
>    napi_complete_done+0x78/0x180 net/core/dev.c:6580
>    tun_get_user+0xa63/0x1120 drivers/net/tun.c:1909
>    tun_chr_write_iter+0x65/0xb0 drivers/net/tun.c:1984
>    vfs_write+0x300/0x420 fs/read_write.c:593
>    ksys_write+0x60/0xd0 fs/read_write.c:686
>    do_syscall_64+0x50/0x1c0 arch/x86/entry/syscall_64.c:63
>    </TASK>
>
> To trigger gso segment in udp_queue_rcv_skb(), we should also set option
> UDP_ENCAP_ESPINUDP to enable udp_sk(sk)->encap_rcv. When the encap_rcv
> hook return 1 in udp_queue_rcv_one_skb(), udp_csum_pull_header() will try
> to pull udphdr, but the skb size has been segmented to gso size, which
> leads to this crash.

Is it correct to access the checksum of a segmented skb?

>
> Only udp has this probloem. Add check for the minimum value of gso size in
> virtio_net_hdr_to_skb().

Why tcp has not this problem?


>
> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
> Fixes: 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing in a tunnel")
> Signed-off-by: Wang Liang <wangliang74@huawei.com>
> ---
>  include/linux/virtio_net.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 02a9f4dc594d..0533101642bd 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -157,11 +157,13 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
>  		unsigned int nh_off = p_off;
>  		struct skb_shared_info *shinfo = skb_shinfo(skb);
> +		u16 min_gso_size = 0;
>
>  		switch (gso_type & ~SKB_GSO_TCP_ECN) {
>  		case SKB_GSO_UDP:
>  			/* UFO may not include transport header in gso_size. */
>  			nh_off -= thlen;
> +			min_gso_size = sizeof(struct udphdr) + 1;
>  			break;
>  		case SKB_GSO_UDP_L4:
>  			if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
> @@ -172,6 +174,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  				return -EINVAL;
>  			if (gso_type != SKB_GSO_UDP_L4)
>  				return -EINVAL;
> +			min_gso_size = sizeof(struct udphdr) + 1;

why +1?

Thanks.

>  			break;
>  		case SKB_GSO_TCPV4:
>  		case SKB_GSO_TCPV6:
> @@ -182,7 +185,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		}
>
>  		/* Kernel has a special handling for GSO_BY_FRAGS. */
> -		if (gso_size == GSO_BY_FRAGS)
> +		if ((gso_size == GSO_BY_FRAGS) || (gso_size < min_gso_size))
>  			return -EINVAL;
>
>  		/* Too small packets are not really GSO ones. */
> --
> 2.34.1
>

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Willem de Bruijn]

Wang Liang wrote:
> 
> 在 2025/7/24 21:29, Willem de Bruijn 写道:
> > Wang Liang wrote:
> >> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> >> size, below crash may happen.
> >>
> > gso_size is the size of the segment payload, excluding the transport
> > header.
> >
> > This is probably not the right approach.
> >
> > Not sure how a GSO skb can be built that is shorter than even the
> > transport header. Maybe an skb_dump of the GSO skb can be elucidating.
> >>   			return -EINVAL;
> >>   
> >>   		/* Too small packets are not really GSO ones. */
> >> -- 
> >> 2.34.1
> >>
> 
> Thanks for your review!

Thanks for the dump and repro.

I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.

> Here is the skb_dump result:
> 
>      skb len=4 headroom=98 headlen=4 tailroom=282
>      mac=(64,14) mac_len=14 net=(78,20) trans=98
>      shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
>      csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)

So this is as expected not the original GSO skb, but a segment,
after udp_rcv_segment from udp_queue_rcv_skb.

It is a packet with skb->data pointing to the transport header, and
only 4B length. So this is an illegal UDP packet with length shorter
than sizeof(struct udphdr).

The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude
that.

It does enter __xfrm4_udp_encap_rcv, which will return 1 because the
pskb_may_pull will fail. There is a negative integer overflow just
before that:

        len = skb->len - sizeof(struct udphdr);
        if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8)))
                return 1;

This is true for all the segments btw, not just the last one. On
return of 1 here, the packet does not enter encap_rcv but gets
passed to the socket as a normal UDP packet:

	/* If it's a keepalive packet, then just eat it.
	 * If it's an encapsulated packet, then pass it to the
	 * IPsec xfrm input.
	 * Returns 0 if skb passed to xfrm or was dropped.
	 * Returns >0 if skb should be passed to UDP.
	 * Returns <0 if skb should be resubmitted (-ret is protocol)
	 */
	int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)

But so the real bug, an skb with 4B in the UDP layer happens before
that.

An skb_dump in udp_queue_rcv_skb of the GSO skb shows

[  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
[  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
[  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
[  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
[  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
[  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
[  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
[  174.152101] dev name=tun0 feat=0x00002000000048c1

And of segs[0] after segmentation

[  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
[  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
[  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
[  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
[  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
[  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
[  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)

So here translen is already 38 - (98-64) == 38 - 34 == 4.

So the bug happens in segmentation.

[ongoing ..]

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Willem de Bruijn]

Willem de Bruijn wrote:
> Wang Liang wrote:
> > 
> > 在 2025/7/24 21:29, Willem de Bruijn 写道:
> > > Wang Liang wrote:
> > >> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> > >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> > >> size, below crash may happen.
> > >>
> > > gso_size is the size of the segment payload, excluding the transport
> > > header.
> > >
> > > This is probably not the right approach.
> > >
> > > Not sure how a GSO skb can be built that is shorter than even the
> > > transport header. Maybe an skb_dump of the GSO skb can be elucidating.
> > >>   			return -EINVAL;
> > >>   
> > >>   		/* Too small packets are not really GSO ones. */
> > >> -- 
> > >> 2.34.1
> > >>
> > 
> > Thanks for your review!
> 
> Thanks for the dump and repro.
> 
> I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.
> 
> > Here is the skb_dump result:
> > 
> >      skb len=4 headroom=98 headlen=4 tailroom=282
> >      mac=(64,14) mac_len=14 net=(78,20) trans=98
> >      shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> >      csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> 
> So this is as expected not the original GSO skb, but a segment,
> after udp_rcv_segment from udp_queue_rcv_skb.
> 
> It is a packet with skb->data pointing to the transport header, and
> only 4B length. So this is an illegal UDP packet with length shorter
> than sizeof(struct udphdr).
> 
> The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude
> that.
> 
> It does enter __xfrm4_udp_encap_rcv, which will return 1 because the
> pskb_may_pull will fail. There is a negative integer overflow just
> before that:
> 
>         len = skb->len - sizeof(struct udphdr);
>         if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8)))
>                 return 1;
> 
> This is true for all the segments btw, not just the last one. On
> return of 1 here, the packet does not enter encap_rcv but gets
> passed to the socket as a normal UDP packet:
> 
> 	/* If it's a keepalive packet, then just eat it.
> 	 * If it's an encapsulated packet, then pass it to the
> 	 * IPsec xfrm input.
> 	 * Returns 0 if skb passed to xfrm or was dropped.
> 	 * Returns >0 if skb should be passed to UDP.
> 	 * Returns <0 if skb should be resubmitted (-ret is protocol)
> 	 */
> 	int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
> 
> But so the real bug, an skb with 4B in the UDP layer happens before
> that.
> 
> An skb_dump in udp_queue_rcv_skb of the GSO skb shows
> 
> [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
> [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
> [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
> [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
> [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
> [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> [  174.152101] dev name=tun0 feat=0x00002000000048c1
> 
> And of segs[0] after segmentation
> 
> [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
> [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
> [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
> [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> 
> So here translen is already 38 - (98-64) == 38 - 34 == 4.
> 
> So the bug happens in segmentation.
> 
> [ongoing ..]

Oh of course, this is udp fragmentation offload (UFO):
VIRTIO_NET_HDR_GSO_UDP.

So only the first packet has an UDP header, and that explains why the
other packets are only 4B.

They are not UDP packets, but they have already entered the UDP stack
due to this being GSO applied in udp_queue_rcv_skb.

That was never intended to be used for UFO. Only for GRO, which does
not build such packets.

Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Jason Wang]

On Mon, Jul 28, 2025 at 1:16 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Willem de Bruijn wrote:
> > Wang Liang wrote:
> > >
> > > 在 2025/7/24 21:29, Willem de Bruijn 写道:
> > > > Wang Liang wrote:
> > > >> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> > > >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> > > >> size, below crash may happen.
> > > >>
> > > > gso_size is the size of the segment payload, excluding the transport
> > > > header.
> > > >
> > > > This is probably not the right approach.
> > > >
> > > > Not sure how a GSO skb can be built that is shorter than even the
> > > > transport header. Maybe an skb_dump of the GSO skb can be elucidating.
> > > >>                          return -EINVAL;
> > > >>
> > > >>                  /* Too small packets are not really GSO ones. */
> > > >> --
> > > >> 2.34.1
> > > >>
> > >
> > > Thanks for your review!
> >
> > Thanks for the dump and repro.
> >
> > I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.
> >
> > > Here is the skb_dump result:
> > >
> > >      skb len=4 headroom=98 headlen=4 tailroom=282
> > >      mac=(64,14) mac_len=14 net=(78,20) trans=98
> > >      shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > >      csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> >
> > So this is as expected not the original GSO skb, but a segment,
> > after udp_rcv_segment from udp_queue_rcv_skb.
> >
> > It is a packet with skb->data pointing to the transport header, and
> > only 4B length. So this is an illegal UDP packet with length shorter
> > than sizeof(struct udphdr).
> >
> > The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude
> > that.
> >
> > It does enter __xfrm4_udp_encap_rcv, which will return 1 because the
> > pskb_may_pull will fail. There is a negative integer overflow just
> > before that:
> >
> >         len = skb->len - sizeof(struct udphdr);
> >         if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8)))
> >                 return 1;
> >
> > This is true for all the segments btw, not just the last one. On
> > return of 1 here, the packet does not enter encap_rcv but gets
> > passed to the socket as a normal UDP packet:
> >
> >       /* If it's a keepalive packet, then just eat it.
> >        * If it's an encapsulated packet, then pass it to the
> >        * IPsec xfrm input.
> >        * Returns 0 if skb passed to xfrm or was dropped.
> >        * Returns >0 if skb should be passed to UDP.
> >        * Returns <0 if skb should be resubmitted (-ret is protocol)
> >        */
> >       int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
> >
> > But so the real bug, an skb with 4B in the UDP layer happens before
> > that.
> >
> > An skb_dump in udp_queue_rcv_skb of the GSO skb shows
> >
> > [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
> > [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
> > [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
> > [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
> > [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> > [  174.152101] dev name=tun0 feat=0x00002000000048c1
> >
> > And of segs[0] after segmentation
> >
> > [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
> > [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> > [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
> > [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> >
> > So here translen is already 38 - (98-64) == 38 - 34 == 4.
> >
> > So the bug happens in segmentation.
> >
> > [ongoing ..]
>
> Oh of course, this is udp fragmentation offload (UFO):
> VIRTIO_NET_HDR_GSO_UDP.
>
> So only the first packet has an UDP header, and that explains why the
> other packets are only 4B.
>
> They are not UDP packets, but they have already entered the UDP stack
> due to this being GSO applied in udp_queue_rcv_skb.
>
> That was never intended to be used for UFO. Only for GRO, which does
> not build such packets.
>
> Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?
>

Just to make sure I understand this. Did you mean to disable UFO for
guest -> host path? If yes, it seems can break some appllication.

Thanks

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Wang Liang]

在 2025/7/28 1:15, Willem de Bruijn 写道:
> Willem de Bruijn wrote:
>> But so the real bug, an skb with 4B in the UDP layer happens before
>> that.
>>
>> An skb_dump in udp_queue_rcv_skb of the GSO skb shows
>>
>> [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
>> [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
>> [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
>> [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
>> [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
>> [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
>> [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
>> [  174.152101] dev name=tun0 feat=0x00002000000048c1
>>
>> And of segs[0] after segmentation
>>
>> [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
>> [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
>> [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
>> [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
>> [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
>> [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
>> [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
>>
>> So here translen is already 38 - (98-64) == 38 - 34 == 4.
>>
>> So the bug happens in segmentation.
>>
>> [ongoing ..]
> Oh of course, this is udp fragmentation offload (UFO):
> VIRTIO_NET_HDR_GSO_UDP.
>
> So only the first packet has an UDP header, and that explains why the
> other packets are only 4B.
>
> They are not UDP packets, but they have already entered the UDP stack
> due to this being GSO applied in udp_queue_rcv_skb.
>
> That was never intended to be used for UFO. Only for GRO, which does
> not build such packets.
>
> Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?


Thank you for your analysis, it is totally right!

After segment in udp_queue_rcv_skb(), these segs are not UDP packets, which
leads the crash.

The packet with VIRTIO_NET_HDR_GSO_UDP type from tun device perhaps should
not enter udp_rcv_segment().

How about not do segment and pass the packet to udp_queue_rcv_one_skb()
directly, like this:

   diff --git a/include/linux/udp.h b/include/linux/udp.h
   index 4e1a672af4c5..0c27e5194657 100644
   --- a/include/linux/udp.h
   +++ b/include/linux/udp.h
   @@ -186,6 +186,9 @@ static inline bool udp_unexpected_gso(struct sock 
*sk, struct sk_buff *skb)
           if (!skb_is_gso(skb))
                   return false;

   +       if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
   +               return false;
   +
           if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
               !udp_test_bit(ACCEPT_L4, sk))
                   return true;

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Sun, Jul 27, 2025 at 11:21 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Jul 28, 2025 at 1:16 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Willem de Bruijn wrote:
> > > Wang Liang wrote:
> > > >
> > > > 在 2025/7/24 21:29, Willem de Bruijn 写道:
> > > > > Wang Liang wrote:
> > > > >> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> > > > >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> > > > >> size, below crash may happen.
> > > > >>
> > > > > gso_size is the size of the segment payload, excluding the transport
> > > > > header.
> > > > >
> > > > > This is probably not the right approach.
> > > > >
> > > > > Not sure how a GSO skb can be built that is shorter than even the
> > > > > transport header. Maybe an skb_dump of the GSO skb can be elucidating.
> > > > >>                          return -EINVAL;
> > > > >>
> > > > >>                  /* Too small packets are not really GSO ones. */
> > > > >> --
> > > > >> 2.34.1
> > > > >>
> > > >
> > > > Thanks for your review!
> > >
> > > Thanks for the dump and repro.
> > >
> > > I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.
> > >
> > > > Here is the skb_dump result:
> > > >
> > > >      skb len=4 headroom=98 headlen=4 tailroom=282
> > > >      mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > >      shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > > >      csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> > >
> > > So this is as expected not the original GSO skb, but a segment,
> > > after udp_rcv_segment from udp_queue_rcv_skb.
> > >
> > > It is a packet with skb->data pointing to the transport header, and
> > > only 4B length. So this is an illegal UDP packet with length shorter
> > > than sizeof(struct udphdr).
> > >
> > > The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude
> > > that.
> > >
> > > It does enter __xfrm4_udp_encap_rcv, which will return 1 because the
> > > pskb_may_pull will fail. There is a negative integer overflow just
> > > before that:
> > >
> > >         len = skb->len - sizeof(struct udphdr);
> > >         if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8)))
> > >                 return 1;
> > >
> > > This is true for all the segments btw, not just the last one. On
> > > return of 1 here, the packet does not enter encap_rcv but gets
> > > passed to the socket as a normal UDP packet:
> > >
> > >       /* If it's a keepalive packet, then just eat it.
> > >        * If it's an encapsulated packet, then pass it to the
> > >        * IPsec xfrm input.
> > >        * Returns 0 if skb passed to xfrm or was dropped.
> > >        * Returns >0 if skb should be passed to UDP.
> > >        * Returns <0 if skb should be resubmitted (-ret is protocol)
> > >        */
> > >       int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
> > >
> > > But so the real bug, an skb with 4B in the UDP layer happens before
> > > that.
> > >
> > > An skb_dump in udp_queue_rcv_skb of the GSO skb shows
> > >
> > > [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
> > > [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
> > > [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
> > > [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > > [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
> > > [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> > > [  174.152101] dev name=tun0 feat=0x00002000000048c1
> > >
> > > And of segs[0] after segmentation
> > >
> > > [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
> > > [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > > [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> > > [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > > [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
> > > [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> > >
> > > So here translen is already 38 - (98-64) == 38 - 34 == 4.
> > >
> > > So the bug happens in segmentation.
> > >
> > > [ongoing ..]
> >
> > Oh of course, this is udp fragmentation offload (UFO):
> > VIRTIO_NET_HDR_GSO_UDP.
> >
> > So only the first packet has an UDP header, and that explains why the
> > other packets are only 4B.
> >
> > They are not UDP packets, but they have already entered the UDP stack
> > due to this being GSO applied in udp_queue_rcv_skb.
> >
> > That was never intended to be used for UFO. Only for GRO, which does
> > not build such packets.
> >
> > Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?
> >
>
> Just to make sure I understand this. Did you mean to disable UFO for
> guest -> host path? If yes, it seems can break some appllication.

No, I mean inside the special segmentation path inside UDP receive.

I know that we have to keep UFO segmentation around because existing
guests may generate those packets and these features cannot be
re-negotiated once enabled, even on migration. But no new kernel
generates UFO packets.

Segmentation inside UDP receive was added when UDP_GRO was added, in
case packets accidentally add up at a local socket receive path that
does not support large packets.

Since GRO never builds UFO packets, such packets should not arrive at
such sockets to begin with.

Evidently we forgot about looping virtio_net_hdr packets. They were
never intended to be supported in this new path, nor clearly have they
ever worked. We just need to mitigate them without crashing.

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Willem de Bruijn]

On Sun, Jul 27, 2025 at 11:36 PM Wang Liang <wangliang74@huawei.com> wrote:
>
>
> 在 2025/7/28 1:15, Willem de Bruijn 写道:
> > Willem de Bruijn wrote:
> >> But so the real bug, an skb with 4B in the UDP layer happens before
> >> that.
> >>
> >> An skb_dump in udp_queue_rcv_skb of the GSO skb shows
> >>
> >> [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
> >> [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
> >> [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
> >> [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
> >> [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> >> [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
> >> [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> >> [  174.152101] dev name=tun0 feat=0x00002000000048c1
> >>
> >> And of segs[0] after segmentation
> >>
> >> [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
> >> [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
> >> [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> >> [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> >> [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> >> [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
> >> [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> >>
> >> So here translen is already 38 - (98-64) == 38 - 34 == 4.
> >>
> >> So the bug happens in segmentation.
> >>
> >> [ongoing ..]
> > Oh of course, this is udp fragmentation offload (UFO):
> > VIRTIO_NET_HDR_GSO_UDP.
> >
> > So only the first packet has an UDP header, and that explains why the
> > other packets are only 4B.
> >
> > They are not UDP packets, but they have already entered the UDP stack
> > due to this being GSO applied in udp_queue_rcv_skb.
> >
> > That was never intended to be used for UFO. Only for GRO, which does
> > not build such packets.
> >
> > Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?
>
>
> Thank you for your analysis, it is totally right!
>
> After segment in udp_queue_rcv_skb(), these segs are not UDP packets, which
> leads the crash.
>
> The packet with VIRTIO_NET_HDR_GSO_UDP type from tun device perhaps should
> not enter udp_rcv_segment().
>
> How about not do segment and pass the packet to udp_queue_rcv_one_skb()
> directly, like this:
>
>    diff --git a/include/linux/udp.h b/include/linux/udp.h
>    index 4e1a672af4c5..0c27e5194657 100644
>    --- a/include/linux/udp.h
>    +++ b/include/linux/udp.h
>    @@ -186,6 +186,9 @@ static inline bool udp_unexpected_gso(struct sock
> *sk, struct sk_buff *skb)
>            if (!skb_is_gso(skb))
>                    return false;
>
>    +       if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
>    +               return false;
>    +
>            if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4 &&
>                !udp_test_bit(ACCEPT_L4, sk))
>                    return true;

That seemingly is what would have happened before introducing
udp_unexpected_gso. The same for SKB_GSO_UDP_L4 packets actually.

It is not in any way an intended use case. And given the longstanding
breakage clearly not actually used.

UDP sockets do not expect GSO packets, so queuing them might confuse them.

Specifically for UFO, it can be argued that they are the same as large
packets that were build by the IP defrag layer.

Still to avoid having to start maintaining an unintentional code path,
including having to likely fix more reports over time and with that
likely add complexity to the real UDP hot path as well, I think we
should instead drop these.

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Jason Wang]

On Mon, Jul 28, 2025 at 11:51 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Sun, Jul 27, 2025 at 11:21 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Mon, Jul 28, 2025 at 1:16 AM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > Willem de Bruijn wrote:
> > > > Wang Liang wrote:
> > > > >
> > > > > 在 2025/7/24 21:29, Willem de Bruijn 写道:
> > > > > > Wang Liang wrote:
> > > > > >> When sending a packet with virtio_net_hdr to tun device, if the gso_type
> > > > > >> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
> > > > > >> size, below crash may happen.
> > > > > >>
> > > > > > gso_size is the size of the segment payload, excluding the transport
> > > > > > header.
> > > > > >
> > > > > > This is probably not the right approach.
> > > > > >
> > > > > > Not sure how a GSO skb can be built that is shorter than even the
> > > > > > transport header. Maybe an skb_dump of the GSO skb can be elucidating.
> > > > > >>                          return -EINVAL;
> > > > > >>
> > > > > >>                  /* Too small packets are not really GSO ones. */
> > > > > >> --
> > > > > >> 2.34.1
> > > > > >>
> > > > >
> > > > > Thanks for your review!
> > > >
> > > > Thanks for the dump and repro.
> > > >
> > > > I can indeed reproduce, only with the UDP_ENCAP_ESPINUDP setsockopt.
> > > >
> > > > > Here is the skb_dump result:
> > > > >
> > > > >      skb len=4 headroom=98 headlen=4 tailroom=282
> > > > >      mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > > >      shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > > > >      csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> > > >
> > > > So this is as expected not the original GSO skb, but a segment,
> > > > after udp_rcv_segment from udp_queue_rcv_skb.
> > > >
> > > > It is a packet with skb->data pointing to the transport header, and
> > > > only 4B length. So this is an illegal UDP packet with length shorter
> > > > than sizeof(struct udphdr).
> > > >
> > > > The packet does not enter xfrm4_gro_udp_encap_rcv, so we can exclude
> > > > that.
> > > >
> > > > It does enter __xfrm4_udp_encap_rcv, which will return 1 because the
> > > > pskb_may_pull will fail. There is a negative integer overflow just
> > > > before that:
> > > >
> > > >         len = skb->len - sizeof(struct udphdr);
> > > >         if (!pskb_may_pull(skb, sizeof(struct udphdr) + min(len, 8)))
> > > >                 return 1;
> > > >
> > > > This is true for all the segments btw, not just the last one. On
> > > > return of 1 here, the packet does not enter encap_rcv but gets
> > > > passed to the socket as a normal UDP packet:
> > > >
> > > >       /* If it's a keepalive packet, then just eat it.
> > > >        * If it's an encapsulated packet, then pass it to the
> > > >        * IPsec xfrm input.
> > > >        * Returns 0 if skb passed to xfrm or was dropped.
> > > >        * Returns >0 if skb should be passed to UDP.
> > > >        * Returns <0 if skb should be resubmitted (-ret is protocol)
> > > >        */
> > > >       int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb)
> > > >
> > > > But so the real bug, an skb with 4B in the UDP layer happens before
> > > > that.
> > > >
> > > > An skb_dump in udp_queue_rcv_skb of the GSO skb shows
> > > >
> > > > [  174.151409] skb len=190 headroom=64 headlen=190 tailroom=66
> > > > [  174.151409] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > > [  174.151409] shinfo(txflags=0 nr_frags=0 gso(size=4 type=65538 segs=0))
> > > > [  174.151409] csum(0x8c start=140 offset=0 ip_summed=3 complete_sw=0 valid=1 level=0)
> > > > [  174.151409] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > > > [  174.151409] priority=0x0 mark=0x0 alloc_cpu=1 vlan_all=0x0
> > > > [  174.151409] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> > > > [  174.152101] dev name=tun0 feat=0x00002000000048c1
> > > >
> > > > And of segs[0] after segmentation
> > > >
> > > > [  103.081442] skb len=38 headroom=64 headlen=38 tailroom=218
> > > > [  103.081442] mac=(64,14) mac_len=14 net=(78,20) trans=98
> > > > [  103.081442] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> > > > [  103.081442] csum(0x8c start=140 offset=0 ip_summed=1 complete_sw=0 valid=1 level=0)
> > > > [  103.081442] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=2 iif=8
> > > > [  103.081442] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0
> > > > [  103.081442] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0)
> > > >
> > > > So here translen is already 38 - (98-64) == 38 - 34 == 4.
> > > >
> > > > So the bug happens in segmentation.
> > > >
> > > > [ongoing ..]
> > >
> > > Oh of course, this is udp fragmentation offload (UFO):
> > > VIRTIO_NET_HDR_GSO_UDP.
> > >
> > > So only the first packet has an UDP header, and that explains why the
> > > other packets are only 4B.
> > >
> > > They are not UDP packets, but they have already entered the UDP stack
> > > due to this being GSO applied in udp_queue_rcv_skb.
> > >
> > > That was never intended to be used for UFO. Only for GRO, which does
> > > not build such packets.
> > >
> > > Maybe we should just drop UFO (SKB_GSO_UDP) packets in this code path?
> > >
> >
> > Just to make sure I understand this. Did you mean to disable UFO for
> > guest -> host path? If yes, it seems can break some appllication.
>
> No, I mean inside the special segmentation path inside UDP receive.
>
> I know that we have to keep UFO segmentation around because existing
> guests may generate those packets and these features cannot be
> re-negotiated once enabled, even on migration. But no new kernel
> generates UFO packets.
>
> Segmentation inside UDP receive was added when UDP_GRO was added, in
> case packets accidentally add up at a local socket receive path that
> does not support large packets.
>
> Since GRO never builds UFO packets, such packets should not arrive at
> such sockets to begin with.
>
> Evidently we forgot about looping virtio_net_hdr packets. They were
> never intended to be supported in this new path, nor clearly have they
> ever worked. We just need to mitigate them without crashing.

Thanks a lot for the clarification. It's clear to me now.

>

Re: [PATCH net] net: check the minimum value of gso size in virtio_net_hdr_to_skb()

[Wang Liang]

在 2025/7/25 16:55, Xuan Zhuo 写道:
> On Thu, 24 Jul 2025 16:30:05 +0800, Wang Liang <wangliang74@huawei.com> wrote:
>> When sending a packet with virtio_net_hdr to tun device, if the gso_type
>> in virtio_net_hdr is SKB_GSO_UDP and the gso_size is less than udphdr
>> size, below crash may happen.
>>
>>    ------------[ cut here ]------------
>>    kernel BUG at net/core/skbuff.c:4572!
>>    Oops: invalid opcode: 0000 [#1] SMP NOPTI
>>    CPU: 0 UID: 0 PID: 62 Comm: mytest Not tainted 6.16.0-rc7 #203 PREEMPT(voluntary)
>>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
>>    RIP: 0010:skb_pull_rcsum+0x8e/0xa0
>>    Code: 00 00 5b c3 cc cc cc cc 8b 93 88 00 00 00 f7 da e8 37 44 38 00 f7 d8 89 83 88 00 00 00 48 8b 83 c8 00 00 00 5b c3 cc cc cc cc <0f> 0b 0f 0b 66 66 2e 0f 1f 84 00 000
>>    RSP: 0018:ffffc900001fba38 EFLAGS: 00000297
>>    RAX: 0000000000000004 RBX: ffff8880040c1000 RCX: ffffc900001fb948
>>    RDX: ffff888003e6d700 RSI: 0000000000000008 RDI: ffff88800411a062
>>    RBP: ffff8880040c1000 R08: 0000000000000000 R09: 0000000000000001
>>    R10: ffff888003606c00 R11: 0000000000000001 R12: 0000000000000000
>>    R13: ffff888004060900 R14: ffff888004050000 R15: ffff888004060900
>>    FS:  000000002406d3c0(0000) GS:ffff888084a19000(0000) knlGS:0000000000000000
>>    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>    CR2: 0000000020000040 CR3: 0000000004007000 CR4: 00000000000006f0
>>    Call Trace:
>>     <TASK>
>>     udp_queue_rcv_one_skb+0x176/0x4b0 net/ipv4/udp.c:2445
>>     udp_queue_rcv_skb+0x155/0x1f0 net/ipv4/udp.c:2475
>>     udp_unicast_rcv_skb+0x71/0x90 net/ipv4/udp.c:2626
>>     __udp4_lib_rcv+0x433/0xb00 net/ipv4/udp.c:2690
>>     ip_protocol_deliver_rcu+0xa6/0x160 net/ipv4/ip_input.c:205
>>     ip_local_deliver_finish+0x72/0x90 net/ipv4/ip_input.c:233
>>     ip_sublist_rcv_finish+0x5f/0x70 net/ipv4/ip_input.c:579
>>     ip_sublist_rcv+0x122/0x1b0 net/ipv4/ip_input.c:636
>>     ip_list_rcv+0xf7/0x130 net/ipv4/ip_input.c:670
>>     __netif_receive_skb_list_core+0x21d/0x240 net/core/dev.c:6067
>>     netif_receive_skb_list_internal+0x186/0x2b0 net/core/dev.c:6210
>>     napi_complete_done+0x78/0x180 net/core/dev.c:6580
>>     tun_get_user+0xa63/0x1120 drivers/net/tun.c:1909
>>     tun_chr_write_iter+0x65/0xb0 drivers/net/tun.c:1984
>>     vfs_write+0x300/0x420 fs/read_write.c:593
>>     ksys_write+0x60/0xd0 fs/read_write.c:686
>>     do_syscall_64+0x50/0x1c0 arch/x86/entry/syscall_64.c:63
>>     </TASK>
>>
>> To trigger gso segment in udp_queue_rcv_skb(), we should also set option
>> UDP_ENCAP_ESPINUDP to enable udp_sk(sk)->encap_rcv. When the encap_rcv
>> hook return 1 in udp_queue_rcv_one_skb(), udp_csum_pull_header() will try
>> to pull udphdr, but the skb size has been segmented to gso size, which
>> leads to this crash.
> Is it correct to access the checksum of a segmented skb?
>
>> Only udp has this probloem. Add check for the minimum value of gso size in
>> virtio_net_hdr_to_skb().
> Why tcp has not this problem?
>
>
>> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
>> Fixes: 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing in a tunnel")
>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>> ---
>>   include/linux/virtio_net.h | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
>> index 02a9f4dc594d..0533101642bd 100644
>> --- a/include/linux/virtio_net.h
>> +++ b/include/linux/virtio_net.h
>> @@ -157,11 +157,13 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>>   		u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
>>   		unsigned int nh_off = p_off;
>>   		struct skb_shared_info *shinfo = skb_shinfo(skb);
>> +		u16 min_gso_size = 0;
>>
>>   		switch (gso_type & ~SKB_GSO_TCP_ECN) {
>>   		case SKB_GSO_UDP:
>>   			/* UFO may not include transport header in gso_size. */
>>   			nh_off -= thlen;
>> +			min_gso_size = sizeof(struct udphdr) + 1;
>>   			break;
>>   		case SKB_GSO_UDP_L4:
>>   			if (!(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM))
>> @@ -172,6 +174,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>>   				return -EINVAL;
>>   			if (gso_type != SKB_GSO_UDP_L4)
>>   				return -EINVAL;
>> +			min_gso_size = sizeof(struct udphdr) + 1;
> why +1?
>
> Thanks.


Hi! Sorry for replying so late.

The first patch should add RFC, because I am not sure it is the best fix
method. Set 'min_gso_size = sizeof(struct udphdr) + 1;' to ensure at least
one byte in payload.

After discussions in mail list, I send a v2 patch, please check it:
https://lore.kernel.org/netdev/20250729123907.3318425-1-wangliang74@huawei.com/T/#u

Thanks.

>>   			break;
>>   		case SKB_GSO_TCPV4:
>>   		case SKB_GSO_TCPV6:
>> @@ -182,7 +185,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
>>   		}
>>
>>   		/* Kernel has a special handling for GSO_BY_FRAGS. */
>> -		if (gso_size == GSO_BY_FRAGS)
>> +		if ((gso_size == GSO_BY_FRAGS) || (gso_size < min_gso_size))
>>   			return -EINVAL;
>>
>>   		/* Too small packets are not really GSO ones. */
>> --
>> 2.34.1
>>