Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] net: virtio_net_hdr_to_skb security check bypassed for TUN devices --
commit 1c7e0adac42db0550277cf456f6845d512993ef1 Author: Paulos Yibelo <[email protected]> net: validate virtio checksum start after network header __virtio_net_hdr_to_skb() rejects a CHECKSUM_PARTIAL start smaller than an estimated minimum network-header length. The comparison currently uses the offset from skb->data rather than the offset from skb_network_header(). > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index c381b916c1b54..a95ad465b9887 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h [ ... ] > @@ -104,7 +104,8 @@ 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) < nh_min_len || > + skb_transport_offset(skb) - skb_network_offset(skb) < > nh_min_len) [Severity: High] Does this check get bypassed for TUN/TAP devices? Looking at drivers/net/tun.c:tun_get_user(), the virtio_net_hdr parsing is invoked before the network header is reset: drivers/net/tun.c:tun_get_user() { ... if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) { atomic_long_inc(&tun->rx_frame_errors); err = -EINVAL; goto free_skb; } ... skb_reset_network_header(skb); ... } Because skb->network_header is uninitialized when __virtio_net_hdr_to_skb() is called, it retains its 0 value from allocation. This causes skb_network_offset(skb) to evaluate to a negative value, such as -64 for prepad bytes. The new check then simplifies to start + prepad < nh_min_len. Since prepad is typically >= 32 and nh_min_len is usually around 20, this comparison is always false. This completely bypasses the security check and leaves TUN/TAP devices vulnerable to the memory corruption this patch intends to fix. While a subsequent patch in the series mitigates the consequence for IPv4 in ip_do_fragment(), are other paths like IPv6 fragmentation via ip6_fragment() still fully exposed to this bypass? > return -EINVAL; > > nh_min_len = skb_transport_offset(skb); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
