Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] net: OOB read in virtio_net_l3_start due to uninitialized mac_header --
commit 7b948a860070205bb3211c20ccd737b2f0e9cfad Author: Michael S. Tsirkin <[email protected]> net: resolve uncommon partial checksums at ingress This commit updates virtio net header parsing to validate checksum offsets and only retain CHECKSUM_PARTIAL when they match parsed TCP or UDP headers. It also introduces early tunnel offset validation. > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index d6466f96cdd00..357115ff0ec2d 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h [ ... ] > @@ -332,10 +550,13 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > inner_th < inner_nh + inner_l3min) > return -EINVAL; > > + if (!virtio_net_tnl_offsets_ok(skb, outer_isv6, outer_th)) > + return -EINVAL; > + [Severity: High] Does this sequence result in accessing an uninitialized MAC header? Tracing the call sequence starting here: virtio_net_hdr_tnl_to_skb() -> virtio_net_tnl_offsets_ok() -> virtio_net_l3_start() Inside virtio_net_l3_start(), if the device type is ARPHRD_ETHER (which is true for IFF_TAP and virtio_net), the code evaluates eth_hdr(skb)->h_proto: include/linux/virtio_net.h:virtio_net_l3_start() { if (skb->dev->type == ARPHRD_ETHER) { if (!pskb_may_pull(skb, ETH_HLEN)) return false; *nhoff = ETH_HLEN; *proto = eth_hdr(skb)->h_proto; ... } However, eth_hdr(skb) strictly evaluates to skb->head + skb->mac_header. Callers like tun_get_user() and virtnet_receive_done() do not appear to initialize the MAC header before calling virtio_net_hdr_tnl_to_skb(). The MAC header is normally initialized via skb_reset_mac_header(skb), which happens in __virtio_net_hdr_to_skb() located just below this check. Because virtio_net_tnl_offsets_ok() is called before __virtio_net_hdr_to_skb(), skb->mac_header retains its default allocation value of 0xFFFF. Could this cause an out-of-bounds memory read (skb->head + 0xFFFF + offsetof(h_proto)) and a potential kernel panic? > /* Let the basic parsing deal with plain GSO features. */ > ret = __virtio_net_hdr_to_skb(skb, hdr, true, > hdr->gso_type & ~gso_tunnel_type); > - if (ret) > + if (ret < 0) > return ret; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
