[PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs

Guoyu Su posted 1 patch 2 weeks ago
net/core/dev.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
[PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Guoyu Su 2 weeks ago
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
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Willem de Bruijn 2 weeks ago
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
>
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Willem de Bruijn 2 weeks ago
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.
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Scars 1 week, 6 days ago
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.
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Willem de Bruijn 1 week, 6 days ago
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.
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Guoyu Su 1 week, 6 days ago
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.
>
>
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Willem de Bruijn 1 week, 5 days ago
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.
> >
> >
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Guoyu Su 1 week, 3 days ago
> 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.
Re: [PATCH net v5] net: use skb_header_pointer() only for DODGY TCPv4 GSO skbs
Posted by Willem de Bruijn 1 week, 2 days ago
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.


~                                                                                                                                               
~                                                                                                                                               
~                                                                                                                                               
~                                                                                                                                               
~
[PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off
Posted by Guoyu Su 1 week, 1 day ago
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
Re: [PATCH net v6] net: use skb_header_pointer() for TCPv4 GSO frag_off
Posted by Willem de Bruijn 1 week, 1 day ago
Guoyu Su wrote:
> 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.

Does not belong in the commit message. It's a diff vs the previous
version.
 
> [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)))

minor: no whitespace in the middle of the variable definition block.
> +		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
>
[PATCH net v7] net: use skb_header_pointer() for TCPv4 GSO frag_off check
Posted by Guoyu Su 1 week ago
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.

[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>
---
v7:
 - Drop revision-diff wording from commit message body.
 - Minor style: keep variable definitions contiguous in gso_features_check().
 - No functional change from v6.

v6: https://lore.kernel.org/netdev/20260326121813.457049-1-yss2813483011xxl@gmail.com/
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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index fc5557062414..831129f2a69b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3821,10 +3821,15 @@ 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;
+		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
Re: [PATCH net v7] net: use skb_header_pointer() for TCPv4 GSO frag_off check
Posted by Willem de Bruijn 1 week ago
Guoyu Su wrote:
> 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.
> 
> [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>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Sashiko asks about similar inner_ip_hdr in skb_gso_has_extension_hdr,
but userspace cannot set skb->encapsulation, so that's a false positive.