[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6835bcbee689a_2e302129482@willemb.c.googlers.com.notmuch>
Date: Tue, 27 May 2025 09:23:10 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Shiming Cheng <shiming.cheng@...iatek.com>,
willemdebruijn.kernel@...il.com,
willemb@...gle.com,
edumazet@...gle.com,
davem@...emloft.net,
kuba@...nel.org,
pabeni@...hat.com,
matthias.bgg@...il.com
Cc: linux-kernel@...r.kernel.org,
netdev@...r.kernel.org,
shiming.cheng@...iatek.com,
lena.wang@...iatek.com
Subject: Re: [PATCH net v3] t: fix udp gso skb_segment after pull from
frag_list
Shiming Cheng wrote:
still subject line: s/t/net/
> Detect invalid geometry due to pull from frag_list, and pass to
> regular skb_segment. If only part of the fraglist payload is pulled
> into head_skb, it will always cause exception when splitting
> skbs by skb_segment. For detailed call stack information, see below.
>
> Valid SKB_GSO_FRAGLIST skbs
> - consist of two or more segments
> - the head_skb holds the protocol headers plus first gso_size
> - one or more frag_list skbs hold exactly one segment
> - all but the last must be gso_size
>
> Optional datapath hooks such as NAT and BPF (bpf_skb_pull_data) can
> modify fraglist skbs, breaking these invariants.
>
> In extreme cases they pull one part of data into skb linear. For UDP,
> this causes three payloads with lengths of (11,11,10) bytes were
> pulled tail to become (12,10,10) bytes.
>
> The skbs no longer meets the above SKB_GSO_FRAGLIST conditions because
> payload was pulled into head_skb, it needs to be linearized before pass
> to regular skb_segment.
>
> skb_segment+0xcd0/0xd14
> __udp_gso_segment+0x334/0x5f4
> udp4_ufo_fragment+0x118/0x15c
> inet_gso_segment+0x164/0x338
> skb_mac_gso_segment+0xc4/0x13c
> __skb_gso_segment+0xc4/0x124
> validate_xmit_skb+0x9c/0x2c0
> validate_xmit_skb_list+0x4c/0x80
> sch_direct_xmit+0x70/0x404
> __dev_queue_xmit+0x64c/0xe5c
> neigh_resolve_output+0x178/0x1c4
> ip_finish_output2+0x37c/0x47c
> __ip_finish_output+0x194/0x240
> ip_finish_output+0x20/0xf4
> ip_output+0x100/0x1a0
> NF_HOOK+0xc4/0x16c
> ip_forward+0x314/0x32c
> ip_rcv+0x90/0x118
> __netif_receive_skb+0x74/0x124
> process_backlog+0xe8/0x1a4
> __napi_poll+0x5c/0x1f8
> net_rx_action+0x154/0x314
> handle_softirqs+0x154/0x4b8
>
> [118.376811] [C201134] rxq0_pus: [name:bug&]kernel BUG at net/core/skbuff.c:4278!
> [118.376829] [C201134] rxq0_pus: [name:traps&]Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> [118.470774] [C201134] rxq0_pus: [name:mrdump&]Kernel Offset: 0x178cc00000 from 0xffffffc008000000
> [118.470810] [C201134] rxq0_pus: [name:mrdump&]PHYS_OFFSET: 0x40000000
> [118.470827] [C201134] rxq0_pus: [name:mrdump&]pstate: 60400005 (nZCv daif +PAN -UAO)
> [118.470848] [C201134] rxq0_pus: [name:mrdump&]pc : [0xffffffd79598aefc] skb_segment+0xcd0/0xd14
> [118.470900] [C201134] rxq0_pus: [name:mrdump&]lr : [0xffffffd79598a5e8] skb_segment+0x3bc/0xd14
> [118.470928] [C201134] rxq0_pus: [name:mrdump&]sp : ffffffc008013770
>
> Fixes: a1e40ac5b5e9 ("net: gso: fix udp gso fraglist segmentation after pull from frag_list")
> Signed-off-by: Shiming Cheng <shiming.cheng@...iatek.com>
> ---
> net/ipv4/udp_offload.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index a5be6e4ed326..ec05bb7d1e22 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -273,6 +273,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> bool copy_dtor;
> __sum16 check;
> __be16 newlen;
> + int ret = 0;
>
> mss = skb_shinfo(gso_skb)->gso_size;
> if (gso_skb->len <= sizeof(*uh) + mss)
> @@ -301,6 +302,9 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
> return __udp_gso_segment_list(gso_skb, features, is_ipv6);
>
> + ret = __skb_linearize(gso_skb);
> + if (ret)
> + return ERR_PTR(ret);
since respin is needed, please add an empty line before this comment
> /* Setup csum, as fraglist skips this in udp4_gro_receive. */
> gso_skb->csum_start = skb_transport_header(gso_skb) - gso_skb->head;
> gso_skb->csum_offset = offsetof(struct udphdr, check);
> --
> 2.45.2
>
Powered by blists - more mailing lists