* [PATCH net] net: clear mangleid_features for SKB_GSO_DODGY TCPv4 @ 2026-03-07 16:29 Guoyu Su 2026-03-07 16:43 ` Eric Dumazet 2026-03-08 8:33 ` [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO Guoyu Su 0 siblings, 2 replies; 18+ messages in thread From: Guoyu Su @ 2026-03-07 16:29 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in netif_skb_features() [1], which originates from gso_features_check(): BUG: KMSAN: uninit-value in gso_features_check net/core/dev.c:3804 [inline] BUG: KMSAN: uninit-value in netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 gso_features_check net/core/dev.c:3804 [inline] netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 validate_xmit_skb+0xb6/0x2400 net/core/dev.c:4003 ... __dev_queue_xmit+0x3016/0x5a50 net/core/dev.c:4795 packet_snd net/packet/af_packet.c:3077 [inline] packet_sendmsg+0x91d9/0xa320 net/packet/af_packet.c:3109 SKB_GSO_DODGY marks packets whose GSO-related header metadata should be treated as untrusted (for example, packets injected via AF_PACKET). gso_features_check() checks IP_DF through iph->frag_off for SKB_GSO_TCPV4 packets. For DODGY packets with untrusted metadata, this may result in a KMSAN uninit-value report. Handle such packets conservatively by clearing mangleid_features when SKB_GSO_DODGY is set, without relying on iph->frag_off. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/68e4b5d8.050a0220.256323.0018.GAE@google.com/T/ Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- net/core/dev.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..9b19154b6f7f 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3808,7 +3808,12 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, struct iphdr *iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); - if (!(iph->frag_off & htons(IP_DF))) + /* SKB_GSO_DODGY packets carry untrusted L3/L4 header metadata. + * Avoid dereferencing IPv4 header fields and conservatively + * clear mangleid support in that case. + */ + if ((skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) || + !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net] net: clear mangleid_features for SKB_GSO_DODGY TCPv4 2026-03-07 16:29 [PATCH net] net: clear mangleid_features for SKB_GSO_DODGY TCPv4 Guoyu Su @ 2026-03-07 16:43 ` Eric Dumazet 2026-03-08 8:33 ` [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO Guoyu Su 1 sibling, 0 replies; 18+ messages in thread From: Eric Dumazet @ 2026-03-07 16:43 UTC (permalink / raw) To: Guoyu Su Cc: davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 On Sat, Mar 7, 2026 at 5:29 PM Guoyu Su <yss2813483011xxl@gmail.com> wrote: > > Syzbot reported a KMSAN uninit-value warning in netif_skb_features() [1], which originates from gso_features_check(): > > BUG: KMSAN: uninit-value in gso_features_check net/core/dev.c:3804 [inline] > BUG: KMSAN: uninit-value in netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 > gso_features_check net/core/dev.c:3804 [inline] > netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 > validate_xmit_skb+0xb6/0x2400 net/core/dev.c:4003 > ... > __dev_queue_xmit+0x3016/0x5a50 net/core/dev.c:4795 > packet_snd net/packet/af_packet.c:3077 [inline] > packet_sendmsg+0x91d9/0xa320 net/packet/af_packet.c:3109 > > SKB_GSO_DODGY marks packets whose GSO-related header metadata should be treated as untrusted (for example, packets injected via AF_PACKET). > > gso_features_check() checks IP_DF through iph->frag_off for SKB_GSO_TCPV4 packets. For DODGY packets with untrusted metadata, this may result in a KMSAN uninit-value report. > > Handle such packets conservatively by clearing mangleid_features when SKB_GSO_DODGY is set, without relying on iph->frag_off. > > [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 > > Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/68e4b5d8.050a0220.256323.0018.GAE@google.com/T/ > Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> Missing FIxes: tag. Please help us. > --- > net/core/dev.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 14a83f2035b9..9b19154b6f7f 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3808,7 +3808,12 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, > struct iphdr *iph = skb->encapsulation ? > inner_ip_hdr(skb) : ip_hdr(skb); > > - if (!(iph->frag_off & htons(IP_DF))) > + /* SKB_GSO_DODGY packets carry untrusted L3/L4 header metadata. > + * Avoid dereferencing IPv4 header fields and conservatively > + * clear mangleid support in that case. > + */ > + if ((skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) || > + !(iph->frag_off & htons(IP_DF))) > features &= ~dev->mangleid_features; > } You are not really fixing the bug, you hide it with another. Please take a look at qdisc_pkt_len_segs_init() for a hint. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO 2026-03-07 16:29 [PATCH net] net: clear mangleid_features for SKB_GSO_DODGY TCPv4 Guoyu Su 2026-03-07 16:43 ` Eric Dumazet @ 2026-03-08 8:33 ` Guoyu Su 2026-03-11 0:48 ` Jakub Kicinski 1 sibling, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-08 8:33 UTC (permalink / raw) To: edumazet, davem, kuba, pabeni Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in netif_skb_features() [1], which originates from gso_features_check(): BUG: KMSAN: uninit-value in gso_features_check net/core/dev.c:3804 [inline] BUG: KMSAN: uninit-value in netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 gso_features_check net/core/dev.c:3804 [inline] netif_skb_features+0x6fb/0x1870 net/core/dev.c:3828 validate_xmit_skb+0xb6/0x2400 net/core/dev.c:4003 ... __dev_queue_xmit+0x3016/0x5a50 net/core/dev.c:4795 packet_snd net/packet/af_packet.c:3077 [inline] packet_sendmsg+0x91d9/0xa320 net/packet/af_packet.c:3109 The current code dereferences iph->frag_off directly to decide whether to clear mangleid_features. For malformed or truncated skb data, this can read invalid/uninitialized header bytes. Use skb_header_pointer() to read the IPv4 header safely from either the outer or inner network header offset. If the header is not accessible, conservatively clear mangleid_features. This keeps the original behavior for valid packets while fixing the unsafe header access pattern. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/68e4b5d8.050a0220.256323.0018.GAE@google.com/T/ Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- v2: - Drop the SKB_GSO_DODGY shortcut. - Use skb_header_pointer() for safe IPv4 header access. - Add missing Fixes tag as requested. v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ net/core/dev.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..c70263654680 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3805,10 +3805,13 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, * segmentation-offloads.rst). */ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { - struct iphdr *iph = skb->encapsulation ? - inner_ip_hdr(skb) : ip_hdr(skb); + struct iphdr _iph; + const struct iphdr *iph; + int nhoff = skb->encapsulation ? + skb_inner_network_offset(skb) : skb_network_offset(skb); - if (!(iph->frag_off & htons(IP_DF))) + iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); + if (!iph || !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO 2026-03-08 8:33 ` [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO Guoyu Su @ 2026-03-11 0:48 ` Jakub Kicinski 2026-03-12 10:43 ` [PATCH net v3] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Jakub Kicinski @ 2026-03-11 0:48 UTC (permalink / raw) To: Guoyu Su Cc: edumazet, davem, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 On Sun, 8 Mar 2026 16:33:19 +0800 Guoyu Su wrote: > - Drop the SKB_GSO_DODGY shortcut. > - Use skb_header_pointer() for safe IPv4 header access. Let's keep the DODGY check and directly compare skb->len to offset + sizeof(*ip)? Some of the callers in the syz repro are probably buggy (HSR?) so they should be fixed rather than ignored. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v3] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-11 0:48 ` Jakub Kicinski @ 2026-03-12 10:43 ` Guoyu Su 2026-03-17 10:22 ` Paolo Abeni 0 siblings, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-12 10:43 UTC (permalink / raw) To: edumazet, davem, kuba, pabeni Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in gso_features_check() called from netif_skb_features() [1]. The current direct skb->len check is not sufficient for SKB_GSO_DODGY packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build a DODGY GSO skb whose total length is large enough, while the IPv4 header is not fully available as initialized linear data for a direct iph->frag_off access. Use skb_header_pointer() to fetch the IPv4 header only for DODGY packets and clear mangleid_features conservatively if the header cannot be accessed or the DF bit is not set. Keep the existing direct ip_hdr()/inner_ip_hdr() access for non-DODGY packets so that buggy internal callers are still exposed. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- v3: - Use skb_header_pointer() only for SKB_GSO_DODGY packets. - Keep direct ip_hdr()/inner_ip_hdr() access for non-DODGY packets. - Fix AF_PACKET/PACKET_VNET_HDR reproducer where skb->len is sufficient but the IPv4 header is not fully available as initialized linear data. v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ net/core/dev.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..f3340d7dd87c 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3805,10 +3805,21 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, * segmentation-offloads.rst). */ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { - struct iphdr *iph = skb->encapsulation ? - inner_ip_hdr(skb) : ip_hdr(skb); + int nhoff = skb->encapsulation ? + skb_inner_network_offset(skb) : + skb_network_offset(skb); + const struct iphdr *iph; - if (!(iph->frag_off & htons(IP_DF))) + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { + struct iphdr _iph; + + iph = nhoff < 0 ? NULL : + skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); + } else { + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); + } + + if (!iph || !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v3] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-12 10:43 ` [PATCH net v3] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs Guoyu Su @ 2026-03-17 10:22 ` Paolo Abeni 2026-03-19 0:54 ` [PATCH net v4] " Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Paolo Abeni @ 2026-03-17 10:22 UTC (permalink / raw) To: Guoyu Su, edumazet, davem, kuba Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 On 3/12/26 11:43 AM, Guoyu Su wrote: > diff --git a/net/core/dev.c b/net/core/dev.c > index 14a83f2035b9..f3340d7dd87c 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3805,10 +3805,21 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, > * segmentation-offloads.rst). > */ > if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { > - struct iphdr *iph = skb->encapsulation ? > - inner_ip_hdr(skb) : ip_hdr(skb); > + int nhoff = skb->encapsulation ? > + skb_inner_network_offset(skb) : > + skb_network_offset(skb); > + const struct iphdr *iph; > > - if (!(iph->frag_off & htons(IP_DF))) > + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { > + struct iphdr _iph; > + > + iph = nhoff < 0 ? NULL : > + skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); > + } else { > + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); > + } > + > + if (!iph || !(iph->frag_off & htons(IP_DF))) > features &= ~dev->mangleid_features; AI review noted the following: Does this code use `_iph` after it goes out of scope? The stack-local variable `_iph` is declared inside the `if (unlikely(...SKB_GSO_DODGY))` block, but `iph` (which may point to `&_iph` when skb_header_pointer() copies the header) is dereferenced via `iph->frag_off` after that block's closing brace, where `_iph` is out of scope. When skb_header_pointer() needs to copy the IP header from paged fragments (the exact scenario this patch targets for DODGY packets from AF_PACKET or HSR), it returns `&_iph`. The subsequent access to `iph->frag_off` then reads from a dead stack variable. All other skb_header_pointer() usage patterns in the kernel declare the buffer at the same scope as the pointer usage. For example, in qdisc_pkt_len_segs_init(): static int qdisc_pkt_len_segs_init(struct sk_buff *skb) { const struct skb_shared_info *shinfo = skb_shinfo(skb); unsigned int hdr_len, mss = shinfo->gso_size; u16 gso_segs = shinfo->gso_segs; const struct iphdr *iph; struct iphdr _iph; // <-- buffer declared in same scope as usage int pkt_len; ... iph = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph); ... if (iph->protocol != IPPROTO_TCP && iph->protocol != IPPROTO_UDP) return -EINVAL; Should `struct iphdr _iph;` be moved to the outer block, next to the `const struct iphdr *iph;` declaration? ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v4] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-17 10:22 ` Paolo Abeni @ 2026-03-19 0:54 ` Guoyu Su 2026-03-19 13:17 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-19 0:54 UTC (permalink / raw) To: edumazet, davem, kuba, pabeni Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in gso_features_check() called from netif_skb_features() [1]. The current direct skb->len check is not sufficient for SKB_GSO_DODGY packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build a DODGY GSO skb whose total length is large enough, while the IPv4 header is not fully available as initialized linear data for a direct iph->frag_off access. Use skb_header_pointer() to fetch the IPv4 header only for DODGY packets and clear mangleid_features conservatively if the header cannot be accessed or the DF bit is not set. Keep the existing direct ip_hdr()/inner_ip_hdr() access for non-DODGY packets so that buggy internal callers are still exposed. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- v4: - Move struct iphdr _iph to the outer scope so a pointer returned by skb_header_pointer() remains valid when iph->frag_off is checked. v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/ v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ net/core/dev.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..d47bf7b1fa99 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3805,10 +3805,20 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, * segmentation-offloads.rst). */ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { - struct iphdr *iph = skb->encapsulation ? - inner_ip_hdr(skb) : ip_hdr(skb); + int nhoff = skb->encapsulation ? + skb_inner_network_offset(skb) : + skb_network_offset(skb); + const struct iphdr *iph; + struct iphdr _iph; + + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { + iph = nhoff < 0 ? NULL : + skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); + } else { + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); + } - if (!(iph->frag_off & htons(IP_DF))) + if (!iph || !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v4] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-19 0:54 ` [PATCH net v4] " Guoyu Su @ 2026-03-19 13:17 ` Willem de Bruijn 2026-03-20 14:14 ` [PATCH net v5] " Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-19 13:17 UTC (permalink / raw) To: Guoyu Su, edumazet, davem, kuba, pabeni Cc: netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Guoyu Su wrote: > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > called from netif_skb_features() [1]. > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > a DODGY GSO skb whose total length is large enough, while the IPv4 > header is not fully available as initialized linear data for a direct > iph->frag_off access. > > Use skb_header_pointer() to fetch the IPv4 header only for DODGY packets > and clear mangleid_features conservatively if the header cannot be > accessed or the DF bit is not set. > > Keep the existing direct ip_hdr()/inner_ip_hdr() access for non-DODGY > packets so that buggy internal callers are still exposed. > > [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 > > Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") > Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 > Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> > --- > v4: > - Move struct iphdr _iph to the outer scope so a pointer returned by > skb_header_pointer() remains valid when iph->frag_off is checked. > > v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/ > v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ > v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ > > net/core/dev.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 14a83f2035b9..d47bf7b1fa99 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3805,10 +3805,20 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, > * segmentation-offloads.rst). > */ > if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { > - struct iphdr *iph = skb->encapsulation ? > - inner_ip_hdr(skb) : ip_hdr(skb); > + int nhoff = skb->encapsulation ? > + skb_inner_network_offset(skb) : > + skb_network_offset(skb); can be moved inside the unlikely branch > + const struct iphdr *iph; > + struct iphdr _iph; > + > + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { > + iph = nhoff < 0 ? NULL : why the negative check? > + skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); > + } else { > + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); > + } > > - if (!(iph->frag_off & htons(IP_DF))) > + if (!iph || !(iph->frag_off & htons(IP_DF))) > features &= ~dev->mangleid_features; > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-19 13:17 ` Willem de Bruijn @ 2026-03-20 14:14 ` Guoyu Su 2026-03-20 19:24 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-20 14:14 UTC (permalink / raw) To: edumazet, davem, kuba, pabeni Cc: willemdebruijn.kernel, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in gso_features_check() called from netif_skb_features() [1]. The current direct skb->len check is not sufficient for SKB_GSO_DODGY packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build a DODGY GSO skb whose total length is large enough, while the IPv4 header is not fully available as initialized linear data for a direct iph->frag_off access. Use skb_header_pointer() to fetch the IPv4 header only for DODGY packets and clear mangleid_features conservatively if the header cannot be accessed or the DF bit is not set. Keep the existing direct ip_hdr()/inner_ip_hdr() access for non-DODGY packets so that buggy internal callers are still exposed. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- v5: - Move nhoff into the SKB_GSO_DODGY branch. - Drop the explicit nhoff < 0 check. v4: https://lore.kernel.org/netdev/20260319005421.14908-1-yss2813483011xxl@gmail.com/ v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/ v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ net/core/dev.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..be0cc69fa534 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3805,10 +3805,20 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, * segmentation-offloads.rst). */ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { - struct iphdr *iph = skb->encapsulation ? - inner_ip_hdr(skb) : ip_hdr(skb); + const struct iphdr *iph; + struct iphdr _iph; - if (!(iph->frag_off & htons(IP_DF))) + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { + int nhoff = skb->encapsulation ? + skb_inner_network_offset(skb) : + skb_network_offset(skb); + + iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); + } else { + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); + } + + if (!iph || !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-20 14:14 ` [PATCH net v5] " Guoyu Su @ 2026-03-20 19:24 ` Willem de Bruijn 2026-03-21 1:36 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-20 19:24 UTC (permalink / raw) To: Guoyu Su, edumazet, davem, kuba, pabeni Cc: willemdebruijn.kernel, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Guoyu Su wrote: > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > called from netif_skb_features() [1]. > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > a DODGY GSO skb whose total length is large enough, while the IPv4 > header is not fully available as initialized linear data for a direct > iph->frag_off access. The fix looks fine, but the AI review of an earlier revision brings up a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths to ensure the network header is fully in skb linear. What kind of packet is this that managed to escape those checks? > Use skb_header_pointer() to fetch the IPv4 header only for DODGY packets > and clear mangleid_features conservatively if the header cannot be > accessed or the DF bit is not set. > > Keep the existing direct ip_hdr()/inner_ip_hdr() access for non-DODGY > packets so that buggy internal callers are still exposed. > > [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 > > Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") > Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 > Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com > Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> > --- > v5: > - Move nhoff into the SKB_GSO_DODGY branch. > - Drop the explicit nhoff < 0 check. > > v4: https://lore.kernel.org/netdev/20260319005421.14908-1-yss2813483011xxl@gmail.com/ > v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/ > v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ > v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ > > net/core/dev.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 14a83f2035b9..be0cc69fa534 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3805,10 +3805,20 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, > * segmentation-offloads.rst). > */ > if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { > - struct iphdr *iph = skb->encapsulation ? > - inner_ip_hdr(skb) : ip_hdr(skb); > + const struct iphdr *iph; > + struct iphdr _iph; > > - if (!(iph->frag_off & htons(IP_DF))) > + if (unlikely(skb_shinfo(skb)->gso_type & SKB_GSO_DODGY)) { > + int nhoff = skb->encapsulation ? > + skb_inner_network_offset(skb) : > + skb_network_offset(skb); > + > + iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); > + } else { > + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); > + } > + > + if (!iph || !(iph->frag_off & htons(IP_DF))) > features &= ~dev->mangleid_features; > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-20 19:24 ` Willem de Bruijn @ 2026-03-21 1:36 ` Willem de Bruijn 2026-03-21 15:31 ` Scars 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-21 1:36 UTC (permalink / raw) To: Willem de Bruijn, Guoyu Su, edumazet, davem, kuba, pabeni Cc: willemdebruijn.kernel, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Willem de Bruijn wrote: > Guoyu Su wrote: > > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > > called from netif_skb_features() [1]. > > > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > > a DODGY GSO skb whose total length is large enough, while the IPv4 > > header is not fully available as initialized linear data for a direct > > iph->frag_off access. > > The fix looks fine, but the AI review of an earlier revision brings up > a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths > to ensure the network header is fully in skb linear. What kind of packet > is this that managed to escape those checks? The packets I got out of the C repro just after virtio_net_hdr_to_skb look as below. [ 76.539562] vnet_hdr: flags=0x75 gso_type=0x1 hlen=0x6a gso_sz=0x416d cstart=0x58 [ 76.539755] skb len=56584 data_len=56476 headroom=4 headlen=108 tailroom=0 [ 76.539755] end-tail=208 mac=(4,76) mac_len=0 net=(80,12) trans=92 [ 76.539755] shinfo(txflags=0 nr_frags=3 gso(size=16749 type=3 segs=0)) [ 76.539755] csum(0x10005c start=92 offset=16 ip_summed=3 complete_sw=0 valid=0 level=0) [ 76.539755] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=0 iif=0 [ 76.539755] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 [ 76.539755] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) [ 76.540713] dev name=ip6gretap0 feat=0x0000000e401d4869 [ 76.540843] sk family=17 type=3 proto=0 Clearly fishy. They do have VIRTIO_NET_HDR_F_NEEDS_CSUM set, so we know which branch they take. skb_reset_mac_header(skb); if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); // start == 88 // needed == 88 + 18 == 106 if (!pskb_may_pull(skb, needed)) return -EINVAL; if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; if (skb_transport_offset(skb) < nh_min_len) return -EINVAL; nh_min_len = skb_transport_offset(skb); // nh_min_len == 88 p_off = nh_min_len + thlen; // p_off == 108 if (!pskb_may_pull(skb, p_off)) return -EINVAL; // headlen == 108 At the end of this headlen == 108, so all of iphdr should be in linear. Since the syz repro requires repeat it is possible that I simply did not capture the right packet, but I don't see the C program vary the packet contents. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-21 1:36 ` Willem de Bruijn @ 2026-03-21 15:31 ` Scars 2026-03-21 20:58 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Scars @ 2026-03-21 15:31 UTC (permalink / raw) To: Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 I instrumented packet_snd(), __virtio_net_hdr_to_skb(), and gso_features_check() while running the C repro. In repeated runs, for the same skb, I consistently observed: - __virtio_net_hdr_to_skb() (NEEDS_CSUM path): skb_transport_offset=88, thlen=20, so p_off=108; pskb_may_pull(..., 108) succeeds (headlen=172). - gso_features_check() on the resulting DODGY TCPv4 skb uses nhoff=skb_network_offset(skb)=172. So the pull checks in __virtio_net_hdr_to_skb() guarantee access up to p_off, but do not guarantee that the header at nhoff is safely linear for direct iph->frag_off dereference. In this run, nhoff==headlen on the observed packets (IPv4 header starts at the linear tail boundary). Using skb_header_pointer() in the DODGY branch avoids this gap. I did not hit a KMSAN report in this rerun (instrumented/patched kernel), but the offset mismatch above was reproducible. Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月21日周六 09:36写道: > > Willem de Bruijn wrote: > > Guoyu Su wrote: > > > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > > > called from netif_skb_features() [1]. > > > > > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > > > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > > > a DODGY GSO skb whose total length is large enough, while the IPv4 > > > header is not fully available as initialized linear data for a direct > > > iph->frag_off access. > > > > The fix looks fine, but the AI review of an earlier revision brings up > > a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths > > to ensure the network header is fully in skb linear. What kind of packet > > is this that managed to escape those checks? > > The packets I got out of the C repro just after virtio_net_hdr_to_skb > look as below. > > [ 76.539562] vnet_hdr: flags=0x75 gso_type=0x1 hlen=0x6a gso_sz=0x416d cstart=0x58 > [ 76.539755] skb len=56584 data_len=56476 headroom=4 headlen=108 tailroom=0 > [ 76.539755] end-tail=208 mac=(4,76) mac_len=0 net=(80,12) trans=92 > [ 76.539755] shinfo(txflags=0 nr_frags=3 gso(size=16749 type=3 segs=0)) > [ 76.539755] csum(0x10005c start=92 offset=16 ip_summed=3 complete_sw=0 valid=0 level=0) > [ 76.539755] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=0 iif=0 > [ 76.539755] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 > [ 76.539755] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > [ 76.540713] dev name=ip6gretap0 feat=0x0000000e401d4869 > [ 76.540843] sk family=17 type=3 proto=0 > > Clearly fishy. They do have VIRTIO_NET_HDR_F_NEEDS_CSUM set, so we > know which branch they take. > > skb_reset_mac_header(skb); > > if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > > // start == 88 > // needed == 88 + 18 == 106 > > if (!pskb_may_pull(skb, needed)) > return -EINVAL; > > if (!skb_partial_csum_set(skb, start, off)) > return -EINVAL; > if (skb_transport_offset(skb) < nh_min_len) > return -EINVAL; > > nh_min_len = skb_transport_offset(skb); > > // nh_min_len == 88 > > p_off = nh_min_len + thlen; > > // p_off == 108 > > if (!pskb_may_pull(skb, p_off)) > return -EINVAL; > > // headlen == 108 > > At the end of this headlen == 108, so all of iphdr should be in > linear. > > Since the syz repro requires repeat it is possible that I simply did > not capture the right packet, but I don't see the C program vary the > packet contents. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-21 15:31 ` Scars @ 2026-03-21 20:58 ` Willem de Bruijn 2026-03-22 4:26 ` Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-21 20:58 UTC (permalink / raw) To: Scars, Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 Scars wrote: > I instrumented packet_snd(), __virtio_net_hdr_to_skb(), and > gso_features_check() while running the C repro. > > In repeated runs, for the same skb, I consistently observed: > - __virtio_net_hdr_to_skb() (NEEDS_CSUM path): > skb_transport_offset=88, thlen=20, so p_off=108; > pskb_may_pull(..., 108) All the above matches the skb_dump from my previous post. > succeeds (headlen=172). My output shows headlen 108. Here we start to diverge. > - gso_features_check() on the resulting DODGY TCPv4 skb uses > nhoff=skb_network_offset(skb)=172. And I see headroom of 4, so mac at 4, skb->network_header at 80 and skb->transport_header at 92. No 172. That part is key. My measurement is in packet_snd right after virtio_net_hdr_to_skb. Where do you see this, and can you perhaps get an skb_dump (NOT full_skb, as these are large, just the header metadata). I don't mean to delay the fix. Just, in general, a preferable fix for these weird user injected packets is to detect and drop as close to kerne entry as possible, meaning in virtio_net_hdr_to_skb, rather than have to make the main datapath robust against crazy packets -- which comes with branches and other overhead on the legitimate hot path. > > So the pull checks in __virtio_net_hdr_to_skb() guarantee access up to > p_off, but do not guarantee that the > header at nhoff is safely linear for direct iph->frag_off dereference. > > In this run, nhoff==headlen on the observed packets (IPv4 header > starts at the linear tail boundary). Using > skb_header_pointer() in the DODGY branch avoids this gap. > > I did not hit a KMSAN report in this rerun (instrumented/patched > kernel), but the offset mismatch above was > reproducible. > > Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月21日周六 09:36写道: > > > > Willem de Bruijn wrote: > > > Guoyu Su wrote: > > > > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > > > > called from netif_skb_features() [1]. > > > > > > > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > > > > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > > > > a DODGY GSO skb whose total length is large enough, while the IPv4 > > > > header is not fully available as initialized linear data for a direct > > > > iph->frag_off access. > > > > > > The fix looks fine, but the AI review of an earlier revision brings up > > > a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths > > > to ensure the network header is fully in skb linear. What kind of packet > > > is this that managed to escape those checks? > > > > The packets I got out of the C repro just after virtio_net_hdr_to_skb > > look as below. > > > > [ 76.539562] vnet_hdr: flags=0x75 gso_type=0x1 hlen=0x6a gso_sz=0x416d cstart=0x58 > > [ 76.539755] skb len=56584 data_len=56476 headroom=4 headlen=108 tailroom=0 > > [ 76.539755] end-tail=208 mac=(4,76) mac_len=0 net=(80,12) trans=92 > > [ 76.539755] shinfo(txflags=0 nr_frags=3 gso(size=16749 type=3 segs=0)) > > [ 76.539755] csum(0x10005c start=92 offset=16 ip_summed=3 complete_sw=0 valid=0 level=0) > > [ 76.539755] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=0 iif=0 > > [ 76.539755] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 > > [ 76.539755] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > > [ 76.540713] dev name=ip6gretap0 feat=0x0000000e401d4869 > > [ 76.540843] sk family=17 type=3 proto=0 > > > > Clearly fishy. They do have VIRTIO_NET_HDR_F_NEEDS_CSUM set, so we > > know which branch they take. > > > > skb_reset_mac_header(skb); > > > > if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > > > > // start == 88 > > // needed == 88 + 18 == 106 > > > > if (!pskb_may_pull(skb, needed)) > > return -EINVAL; > > > > if (!skb_partial_csum_set(skb, start, off)) > > return -EINVAL; > > if (skb_transport_offset(skb) < nh_min_len) > > return -EINVAL; > > > > nh_min_len = skb_transport_offset(skb); > > > > // nh_min_len == 88 > > > > p_off = nh_min_len + thlen; > > > > // p_off == 108 > > > > if (!pskb_may_pull(skb, p_off)) > > return -EINVAL; > > > > // headlen == 108 > > > > At the end of this headlen == 108, so all of iphdr should be in > > linear. > > > > Since the syz repro requires repeat it is possible that I simply did > > not capture the right packet, but I don't see the C program vary the > > packet contents. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-21 20:58 ` Willem de Bruijn @ 2026-03-22 4:26 ` Guoyu Su 2026-03-23 3:36 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-22 4:26 UTC (permalink / raw) To: Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 Thanks Willem, this is a good point. I reran with instrumentation at two exact points: 1) packet_snd(), immediately after virtio_net_hdr_to_skb() returns (net/packet/af_packet.c) 2) gso_features_check(), in the SKB_GSO_DODGY branch before frag_off access (net/core/dev.c) For the same skb, I consistently see (first 20 dumps): - pkt_after_vnet: skb=... len=56584 headlen=172 data_len=56412 netoff=172 transoff=88 gso_type=0x3 skb_dump: headroom=4, mac=(4,172), trans=92 - gso_dodgy: skb=... nhoff=172 headlen=172 netoff=172 transoff=88 So in this run, coverage up to p_off on the transport-side does not imply safe direct access at nhoff on the network-side (nhoff/headlen are both 172 here). I agree that validating/dropping malformed packets as early as possible in virtio_net_hdr_to_skb() would be preferable if we can make that check precise. This patch addresses the observed safety gap at gso_features_check() for DODGY packets in the current path. If helpful, I can share more skb_dump snippets / full serial log. Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月22日周日 04:58写道: > > Scars wrote: > > I instrumented packet_snd(), __virtio_net_hdr_to_skb(), and > > gso_features_check() while running the C repro. > > > > In repeated runs, for the same skb, I consistently observed: > > - __virtio_net_hdr_to_skb() (NEEDS_CSUM path): > > skb_transport_offset=88, thlen=20, so p_off=108; > > pskb_may_pull(..., 108) > > All the above matches the skb_dump from my previous post. > > > succeeds (headlen=172). > > My output shows headlen 108. Here we start to diverge. > > > - gso_features_check() on the resulting DODGY TCPv4 skb uses > > nhoff=skb_network_offset(skb)=172. > > And I see headroom of 4, so mac at 4, skb->network_header at 80 and > skb->transport_header at 92. No 172. > > That part is key. My measurement is in packet_snd right after > virtio_net_hdr_to_skb. Where do you see this, and can you perhaps get > an skb_dump (NOT full_skb, as these are large, just the header > metadata). > > I don't mean to delay the fix. Just, in general, a preferable fix for > these weird user injected packets is to detect and drop as close to > kerne entry as possible, meaning in virtio_net_hdr_to_skb, rather than > have to make the main datapath robust against crazy packets -- which > comes with branches and other overhead on the legitimate hot path. > > > > > > So the pull checks in __virtio_net_hdr_to_skb() guarantee access up to > > p_off, but do not guarantee that the > > header at nhoff is safely linear for direct iph->frag_off dereference. > > > > In this run, nhoff==headlen on the observed packets (IPv4 header > > starts at the linear tail boundary). Using > > skb_header_pointer() in the DODGY branch avoids this gap. > > > > I did not hit a KMSAN report in this rerun (instrumented/patched > > kernel), but the offset mismatch above was > > reproducible. > > > > Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月21日周六 09:36写道: > > > > > > Willem de Bruijn wrote: > > > > Guoyu Su wrote: > > > > > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > > > > > called from netif_skb_features() [1]. > > > > > > > > > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > > > > > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > > > > > a DODGY GSO skb whose total length is large enough, while the IPv4 > > > > > header is not fully available as initialized linear data for a direct > > > > > iph->frag_off access. > > > > > > > > The fix looks fine, but the AI review of an earlier revision brings up > > > > a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths > > > > to ensure the network header is fully in skb linear. What kind of packet > > > > is this that managed to escape those checks? > > > > > > The packets I got out of the C repro just after virtio_net_hdr_to_skb > > > look as below. > > > > > > [ 76.539562] vnet_hdr: flags=0x75 gso_type=0x1 hlen=0x6a gso_sz=0x416d cstart=0x58 > > > [ 76.539755] skb len=56584 data_len=56476 headroom=4 headlen=108 tailroom=0 > > > [ 76.539755] end-tail=208 mac=(4,76) mac_len=0 net=(80,12) trans=92 > > > [ 76.539755] shinfo(txflags=0 nr_frags=3 gso(size=16749 type=3 segs=0)) > > > [ 76.539755] csum(0x10005c start=92 offset=16 ip_summed=3 complete_sw=0 valid=0 level=0) > > > [ 76.539755] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=0 iif=0 > > > [ 76.539755] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 > > > [ 76.539755] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > > > [ 76.540713] dev name=ip6gretap0 feat=0x0000000e401d4869 > > > [ 76.540843] sk family=17 type=3 proto=0 > > > > > > Clearly fishy. They do have VIRTIO_NET_HDR_F_NEEDS_CSUM set, so we > > > know which branch they take. > > > > > > skb_reset_mac_header(skb); > > > > > > if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > > > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > > > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > > > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > > > > > > // start == 88 > > > // needed == 88 + 18 == 106 > > > > > > if (!pskb_may_pull(skb, needed)) > > > return -EINVAL; > > > > > > if (!skb_partial_csum_set(skb, start, off)) > > > return -EINVAL; > > > if (skb_transport_offset(skb) < nh_min_len) > > > return -EINVAL; > > > > > > nh_min_len = skb_transport_offset(skb); > > > > > > // nh_min_len == 88 > > > > > > p_off = nh_min_len + thlen; > > > > > > // p_off == 108 > > > > > > if (!pskb_may_pull(skb, p_off)) > > > return -EINVAL; > > > > > > // headlen == 108 > > > > > > At the end of this headlen == 108, so all of iphdr should be in > > > linear. > > > > > > Since the syz repro requires repeat it is possible that I simply did > > > not capture the right packet, but I don't see the C program vary the > > > packet contents. > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-22 4:26 ` Guoyu Su @ 2026-03-23 3:36 ` Willem de Bruijn 2026-03-24 10:40 ` Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-23 3:36 UTC (permalink / raw) To: Guoyu Su, Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 Guoyu Su wrote: > Thanks Willem, this is a good point. > > I reran with instrumentation at two exact points: > 1) packet_snd(), immediately after virtio_net_hdr_to_skb() returns > (net/packet/af_packet.c) > 2) gso_features_check(), in the SKB_GSO_DODGY branch before frag_off access > (net/core/dev.c) > > For the same skb, I consistently see (first 20 dumps): > - pkt_after_vnet: > skb=... len=56584 headlen=172 data_len=56412 netoff=172 transoff=88 > gso_type=0x3 > skb_dump: headroom=4, mac=(4,172), trans=92 > - gso_dodgy: > skb=... nhoff=172 headlen=172 netoff=172 transoff=88 > > So in this run, coverage up to p_off on the transport-side does not imply > safe direct access at nhoff on the network-side (nhoff/headlen are > both 172 here). Perhaps you're running a different repro from the one I used. Which is the C repro from the run at commit ca4ee40bf13d. I see that the virtio_net_hdr has hdr_len 106 and csum_start 88. Those are fine. Same for your repro? The question is how skb->network_header can be greater than skb->transport_header right after virtio_net_hdr_to_skb. And whether this can be a sanity test to drop clearly malformed packets. E.g., @@ -105,8 +108,12 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, return -EINVAL; if (skb_transport_offset(skb) < nh_min_len) return -EINVAL; + if (skb_transport_offset(skb) < skb_network_offset(skb) + nh_min_len) + return -EINVAL; As far as I can see network_header is set entirely in packet_snd, not updated in virtio_net_hdr_to_skb in this path. It seems that hard_header_len for this device is 76. That is part of the answer. It is an ip6gretap device, so this is the encap hlen. > I agree that validating/dropping malformed packets as early as possible in > virtio_net_hdr_to_skb() would be preferable if we can make that check precise. > This patch addresses the observed safety gap at gso_features_check() for DODGY > packets in the current path. > > If helpful, I can share more skb_dump snippets / full serial log. Friendly reminder to not top post https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions > Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月22日周日 04:58写道: > > > > Scars wrote: > > > I instrumented packet_snd(), __virtio_net_hdr_to_skb(), and > > > gso_features_check() while running the C repro. > > > > > > In repeated runs, for the same skb, I consistently observed: > > > - __virtio_net_hdr_to_skb() (NEEDS_CSUM path): > > > skb_transport_offset=88, thlen=20, so p_off=108; > > > pskb_may_pull(..., 108) > > > > All the above matches the skb_dump from my previous post. > > > > > succeeds (headlen=172). > > > > My output shows headlen 108. Here we start to diverge. > > > > > - gso_features_check() on the resulting DODGY TCPv4 skb uses > > > nhoff=skb_network_offset(skb)=172. > > > > And I see headroom of 4, so mac at 4, skb->network_header at 80 and > > skb->transport_header at 92. No 172. > > > > That part is key. My measurement is in packet_snd right after > > virtio_net_hdr_to_skb. Where do you see this, and can you perhaps get > > an skb_dump (NOT full_skb, as these are large, just the header > > metadata). > > > > I don't mean to delay the fix. Just, in general, a preferable fix for > > these weird user injected packets is to detect and drop as close to > > kerne entry as possible, meaning in virtio_net_hdr_to_skb, rather than > > have to make the main datapath robust against crazy packets -- which > > comes with branches and other overhead on the legitimate hot path. > > > > > > > > > > So the pull checks in __virtio_net_hdr_to_skb() guarantee access up to > > > p_off, but do not guarantee that the > > > header at nhoff is safely linear for direct iph->frag_off dereference. > > > > > > In this run, nhoff==headlen on the observed packets (IPv4 header > > > starts at the linear tail boundary). Using > > > skb_header_pointer() in the DODGY branch avoids this gap. > > > > > > I did not hit a KMSAN report in this rerun (instrumented/patched > > > kernel), but the offset mismatch above was > > > reproducible. > > > > > > Willem de Bruijn <willemdebruijn.kernel@gmail.com> 于2026年3月21日周六 09:36写道: > > > > > > > > Willem de Bruijn wrote: > > > > > Guoyu Su wrote: > > > > > > Syzbot reported a KMSAN uninit-value warning in gso_features_check() > > > > > > called from netif_skb_features() [1]. > > > > > > > > > > > > The current direct skb->len check is not sufficient for SKB_GSO_DODGY > > > > > > packets. In the AF_PACKET/PACKET_VNET_HDR path, packet_snd() can build > > > > > > a DODGY GSO skb whose total length is large enough, while the IPv4 > > > > > > header is not fully available as initialized linear data for a direct > > > > > > iph->frag_off access. > > > > > > > > > > The fix looks fine, but the AI review of an earlier revision brings up > > > > > a good point: __virtio_net_hdr_to_skb calls pskb_may_pull in all paths > > > > > to ensure the network header is fully in skb linear. What kind of packet > > > > > is this that managed to escape those checks? > > > > > > > > The packets I got out of the C repro just after virtio_net_hdr_to_skb > > > > look as below. > > > > > > > > [ 76.539562] vnet_hdr: flags=0x75 gso_type=0x1 hlen=0x6a gso_sz=0x416d cstart=0x58 > > > > [ 76.539755] skb len=56584 data_len=56476 headroom=4 headlen=108 tailroom=0 > > > > [ 76.539755] end-tail=208 mac=(4,76) mac_len=0 net=(80,12) trans=92 > > > > [ 76.539755] shinfo(txflags=0 nr_frags=3 gso(size=16749 type=3 segs=0)) > > > > [ 76.539755] csum(0x10005c start=92 offset=16 ip_summed=3 complete_sw=0 valid=0 level=0) > > > > [ 76.539755] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=0 iif=0 > > > > [ 76.539755] priority=0x0 mark=0x0 alloc_cpu=0 vlan_all=0x0 > > > > [ 76.539755] encapsulation=0 inner(proto=0x0000, mac=0, net=0, trans=0) > > > > [ 76.540713] dev name=ip6gretap0 feat=0x0000000e401d4869 > > > > [ 76.540843] sk family=17 type=3 proto=0 > > > > > > > > Clearly fishy. They do have VIRTIO_NET_HDR_F_NEEDS_CSUM set, so we > > > > know which branch they take. > > > > > > > > skb_reset_mac_header(skb); > > > > > > > > if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > > > > u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start); > > > > u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset); > > > > u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16)); > > > > > > > > // start == 88 > > > > // needed == 88 + 18 == 106 > > > > > > > > if (!pskb_may_pull(skb, needed)) > > > > return -EINVAL; > > > > > > > > if (!skb_partial_csum_set(skb, start, off)) > > > > return -EINVAL; > > > > if (skb_transport_offset(skb) < nh_min_len) > > > > return -EINVAL; > > > > > > > > nh_min_len = skb_transport_offset(skb); > > > > > > > > // nh_min_len == 88 > > > > > > > > p_off = nh_min_len + thlen; > > > > > > > > // p_off == 108 > > > > > > > > if (!pskb_may_pull(skb, p_off)) > > > > return -EINVAL; > > > > > > > > // headlen == 108 > > > > > > > > At the end of this headlen == 108, so all of iphdr should be in > > > > linear. > > > > > > > > Since the syz repro requires repeat it is possible that I simply did > > > > not capture the right packet, but I don't see the C program vary the > > > > packet contents. > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-23 3:36 ` Willem de Bruijn @ 2026-03-24 10:40 ` Guoyu Su 2026-03-26 3:12 ` Willem de Bruijn 0 siblings, 1 reply; 18+ messages in thread From: Guoyu Su @ 2026-03-24 10:40 UTC (permalink / raw) To: Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 > Perhaps you're running a different repro from the one I used. Which is > the C repro from the run at commit ca4ee40bf13d. I reran with the exact ReproC from that run: https://syzkaller.appspot.com/text?tag=ReproC&x=10e3fe5a580000 (commit ca4ee40bf13d), rebuilt locally, and reran. > I see that the virtio_net_hdr has hdr_len 106 and csum_start 88. > Those are fine. Same for your repro? Yes, same in my run as well: vnet_hlen=106, vnet_csum_start=88 (first 20 dumps are consistent). > The question is how skb->network_header can be greater than > skb->transport_header right after virtio_net_hdr_to_skb. From my instrumentation on the same skb in packet_snd(): - packet_parse_headers() sets netoff from device L2 layout: hard_hlen=172 on ip6gretap0, so netoff=172. - then virtio NEEDS_CSUM path sets transoff from csum_start: transoff=88 (p_off=108). So for the same skb I consistently see: - snd_pre_parse / snd_post_parse: headlen=172, netoff=172 - virtio_needs_csum: start=88, needed=108, p_off=108, transoff=88 - gso_dodgy: nhoff=172, headlen=172, netoff=172, transoff=88 I agree early sanity/drop in virtio_net_hdr_to_skb() is preferable if we can make the check precise. The current patch is only to make gso_features_check() safe for DODGY packets in the current path. For reference, skb_dump metadata on that same skb shows: headroom=4, mac=(4,172), trans=92, net=(176,-84), headlen=172. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs 2026-03-24 10:40 ` Guoyu Su @ 2026-03-26 3:12 ` Willem de Bruijn 2026-03-26 12:18 ` [PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off Guoyu Su 0 siblings, 1 reply; 18+ messages in thread From: Willem de Bruijn @ 2026-03-26 3:12 UTC (permalink / raw) To: Guoyu Su, Willem de Bruijn Cc: edumazet, davem, kuba, pabeni, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407 Guoyu Su wrote: > > Perhaps you're running a different repro from the one I used. Which is > > the C repro from the run at commit ca4ee40bf13d. Thanks for verifying. > I reran with the exact ReproC from that run: > https://syzkaller.appspot.com/text?tag=ReproC&x=10e3fe5a580000 > (commit ca4ee40bf13d), rebuilt locally, and reran. > > > I see that the virtio_net_hdr has hdr_len 106 and csum_start 88. > > Those are fine. Same for your repro? > > Yes, same in my run as well: > vnet_hlen=106, vnet_csum_start=88 (first 20 dumps are consistent). > > > The question is how skb->network_header can be greater than > > skb->transport_header right after virtio_net_hdr_to_skb. > > From my instrumentation on the same skb in packet_snd(): > - packet_parse_headers() sets netoff from device L2 layout: > hard_hlen=172 on ip6gretap0, so netoff=172. I don't see this, but hard coding it gets the same issue. More importantly, I took a closer look at a fix. Unfortunately skb_network_offset cannot be trusted for link layers with variable length headers. With SOCK_RAW it is the worst case hard_header_length. PF_PACKET is network layer agnostic, and with SOCK_RAW on variable length link layer packets, nothing communicates this. So, a straightforward check like this may have false positives where a valid packet is shorter than this worst case estimation of network offset. @@ -103,7 +103,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, if (!skb_partial_csum_set(skb, start, off)) return -EINVAL; - if (skb_transport_offset(skb) < nh_min_len) + if (skb_transport_offset(skb) < skb_network_offset(skb) + nh_min_len) As a result, I don't see any test we can do at this point that will not have false positives. Only for GSO packets is there downstream code that requires the network header and that assumes this header starts at skb->network_header. __skb_gso_segment calls skb_reset_mac_len(skb) and parses the headers in skb_mac_gso_segment and skb_network_protocol robustly using skb_header_pointer and pskb_may_pull. We can at this entry point anticipate reaching that code and add an extra branch if gso_type. But might as well just make robust the one access in the GSO path that is not yet. So in short your original approach is probably preferable. Please do add a Link: to this thread. And one more thing: skb_header_pointer already takes the fast path of just returning the offset if within linear. No need to special case the DODGY vs non-DODGY path. ~ ~ ~ ~ ~ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off 2026-03-26 3:12 ` Willem de Bruijn @ 2026-03-26 12:18 ` Guoyu Su 0 siblings, 0 replies; 18+ messages in thread From: Guoyu Su @ 2026-03-26 12:18 UTC (permalink / raw) To: edumazet, davem, kuba, pabeni Cc: willemdebruijn.kernel, netdev, horms, linux-kernel, syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su Syzbot reported a KMSAN uninit-value warning in gso_features_check() called from netif_skb_features() [1]. gso_features_check() reads iph->frag_off to decide whether to clear mangleid_features. Accessing the IPv4 header via ip_hdr()/inner_ip_hdr() can rely on skb header offsets that are not always safe for direct dereference on packets injected from PF_PACKET paths. Use skb_header_pointer() for the TCPv4 frag_off check so the header read is robust whether data is already linear or needs copying. This also removes the SKB_GSO_DODGY special casing: skb_header_pointer() already fast-paths linear data, so a separate direct-access path is not needed. [1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.1a9f35039caab@gmail.com/ Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID") Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407 Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com> --- v6: - Use skb_header_pointer() for both DODGY and non-DODGY TCPv4 GSO packets in gso_features_check(). - Drop the SKB_GSO_DODGY special-casing for IPv4 header access. v5: https://lore.kernel.org/netdev/20260320141459.9691-1-yss2813483011xxl@gmail.com/ v4: https://lore.kernel.org/netdev/20260319005421.14908-1-yss2813483011xxl@gmail.com/ v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/ v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/ v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/ net/core/dev.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 14a83f2035b9..8a15ca67cfed 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3805,10 +3805,16 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb, * segmentation-offloads.rst). */ if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) { - struct iphdr *iph = skb->encapsulation ? - inner_ip_hdr(skb) : ip_hdr(skb); + const struct iphdr *iph; + struct iphdr _iph; - if (!(iph->frag_off & htons(IP_DF))) + int nhoff = skb->encapsulation ? + skb_inner_network_offset(skb) : + skb_network_offset(skb); + + iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph); + + if (!iph || !(iph->frag_off & htons(IP_DF))) features &= ~dev->mangleid_features; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-03-26 12:19 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-07 16:29 [PATCH net] net: clear mangleid_features for SKB_GSO_DODGY TCPv4 Guoyu Su 2026-03-07 16:43 ` Eric Dumazet 2026-03-08 8:33 ` [PATCH net v2] net: use skb_header_pointer() in gso_features_check() for TCPv4 GSO Guoyu Su 2026-03-11 0:48 ` Jakub Kicinski 2026-03-12 10:43 ` [PATCH net v3] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs Guoyu Su 2026-03-17 10:22 ` Paolo Abeni 2026-03-19 0:54 ` [PATCH net v4] " Guoyu Su 2026-03-19 13:17 ` Willem de Bruijn 2026-03-20 14:14 ` [PATCH net v5] " Guoyu Su 2026-03-20 19:24 ` Willem de Bruijn 2026-03-21 1:36 ` Willem de Bruijn 2026-03-21 15:31 ` Scars 2026-03-21 20:58 ` Willem de Bruijn 2026-03-22 4:26 ` Guoyu Su 2026-03-23 3:36 ` Willem de Bruijn 2026-03-24 10:40 ` Guoyu Su 2026-03-26 3:12 ` Willem de Bruijn 2026-03-26 12:18 ` [PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off Guoyu Su
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox