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

[Scars]

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@xxxxxxxxx> 于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.