lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <17042f3d-ce75-4be4-a53e-3d57907488e8@huawei.com>
Date: Tue, 29 Jul 2025 20:22:46 +0800
From: Wang Liang <wangliang74@...wei.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
CC: <yuehaibing@...wei.com>, <zhangchangzhong@...wei.com>,
	<netdev@...r.kernel.org>, <virtualization@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <mst@...hat.com>, <jasowang@...hat.com>,
	<eperezma@...hat.com>, <pabeni@...hat.com>, <davem@...emloft.net>,
	<willemb@...gle.com>, <atenart@...nel.org>
Subject: Re: [PATCH net] net: check the minimum value of gso size in
 virtio_net_hdr_to_skb()


在 2025/7/25 16:55, Xuan Zhuo 写道:
> On Thu, 24 Jul 2025 16:30:05 +0800, Wang Liang <wangliang74@...wei.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@...wei.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
>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ