[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5fa8a2db-d79b-4bcc-a386-03f3706c319f@huawei.com>
Date: Wed, 30 Jul 2025 18:08:26 +0800
From: Wang Liang <wangliang74@...wei.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<horms@...nel.org>, <atenart@...nel.org>
CC: <yuehaibing@...wei.com>, <zhangchangzhong@...wei.com>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net v2] net: drop gso udp packets in udp_rcv_segment()
在 2025/7/30 1:09, Willem de Bruijn 写道:
> Can limit to drop UFO packets, as other GSO packets will get segmented
> correctly.
>
> 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!
>> 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.
>>
>> Previous commit cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
>> introduces segmentation in UDP receive path only for GRO, which was never
>> intended to be used for UFO, so drop gso udp packets in udp_rcv_segment().
>>
>> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
>> Fixes: 3d010c8031e3 ("udp: do not accept non-tunnel GSO skbs landing in a tunnel")
> Only one Fixes tag, to know where to backport too.
>
> The segmentation on receive is introduced in the first commit. I'd
> keep only that.
>
> And please add a Link: to the email thread with the analysis of the
> bug, your v1 (above the ---).
Thanks very much for your suggestions! They are helpful and clear.
I have send a v3 patch, please check it. Thanks.
------
Best regards
Wang Liang
>> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@...il.com>
>> Signed-off-by: Wang Liang <wangliang74@...wei.com>
>> ---
>> v1: https://lore.kernel.org/netdev/20250724083005.3918375-1-wangliang74@huawei.com/
>> v2: Drop ufo packets instead of checking min gso size.
>> ---
>> include/net/udp.h | 22 ++++++++++++++++------
>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/udp.h b/include/net/udp.h
>> index a772510b2aa5..e3fcda71f6c1 100644
>> --- a/include/net/udp.h
>> +++ b/include/net/udp.h
>> @@ -587,6 +587,14 @@ static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
>> {
>> netdev_features_t features = NETIF_F_SG;
>> struct sk_buff *segs;
>> + int drop_count = 1;
> This is rare, can move initialization into the branch.
>> +
>> + /*
>> + * Segmentation in UDP receive path is only for UDP GRO, drop udp
>> + * fragmentation offload (UFO) packets.
>> + */
>> + if (skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP_L4))
> Only SKB_GSO_UDP.
>
> The purpose of this function is to correctly segment SKB_GSO_UDP_L4
> packets.
>
>> + goto drop;
>>
>> /* Avoid csum recalculation by skb_segment unless userspace explicitly
>> * asks for the final checksum values
>> @@ -610,16 +618,18 @@ static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
>> */
>> segs = __skb_gso_segment(skb, features, false);
>> if (IS_ERR_OR_NULL(segs)) {
>> - int segs_nr = skb_shinfo(skb)->gso_segs;
>> -
>> - atomic_add(segs_nr, &sk->sk_drops);
>> - SNMP_ADD_STATS(__UDPX_MIB(sk, ipv4), UDP_MIB_INERRORS, segs_nr);
>> - kfree_skb(skb);
>> - return NULL;
>> + drop_count = skb_shinfo(skb)->gso_segs;
>> + goto drop;
>> }
>>
>> consume_skb(skb);
>> return segs;
>> +
>> +drop:
>> + atomic_add(drop_count, &sk->sk_drops);
>> + SNMP_ADD_STATS(__UDPX_MIB(sk, ipv4), UDP_MIB_INERRORS, drop_count);
>> + kfree_skb(skb);
>> + return NULL;
>> }
>>
>> static inline void udp_post_segment_fix_csum(struct sk_buff *skb)
>> --
>> 2.34.1
>>
>
Powered by blists - more mailing lists