Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] net: virtio_net_csum_partial_ok() fails to bypass virtio_net_l3_start() --
commit 559fb10ce506a1c7276bf12adfdeed16a12a4f0c Author: Michael S. Tsirkin <[email protected]> net: add a sysctl to opt out of virtio net hdr checksum validation This commit adds a per-netns sysctl net.core.virtio_net_hdr_csum_check guarded by a static key to allow opting out of ingress virtio_net_hdr checksum metadata validation. > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > index 97b554ef7123f..25aa95c11e31a 100644 > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h [ ... ] > @@ -179,6 +193,12 @@ virtio_net_csum_partial_ok_at_offset(struct sk_buff > *skb, u8 hdr_gso_type, > unsigned int thoff; > int ip_proto; > > + /* Failsafe: a namespace opted out of the validation, > + * trust the checksum metadata as before. > + */ > + if (virtio_net_hdr_csum_check_disabled(skb)) > + return true; > + > if (!virtio_net_l4_start(skb, nhoff, proto, &thoff, &ip_proto)) > return false; > > @@ -516,6 +536,9 @@ virtio_net_tnl_offsets_ok(struct sk_buff *skb, bool > outer_isv6, > unsigned int thoff; > int ip_proto; > > + if (virtio_net_hdr_csum_check_disabled(skb)) > + return true; > + > if (!virtio_net_l3_start(skb, &nhoff, &proto)) > return false; > if (outer_isv6 != (proto == htons(ETH_P_IPV6))) [Severity: Medium] In virtio_net_tnl_offsets_ok(), virtio_net_hdr_csum_check_disabled(skb) is checked before calling virtio_net_l3_start(). However, in the non-tunnel path, virtio_net_hdr_to_skb() calls virtio_net_csum_partial_ok(), which calls virtio_net_l3_start() before reaching the check in virtio_net_csum_partial_ok_at_offset(): include/linux/virtio_net.h:virtio_net_csum_partial_ok() { int nhoff; __be16 proto; if (!virtio_net_l3_start(skb, &nhoff, &proto)) return false; return virtio_net_csum_partial_ok_at_offset(skb, hdr_gso_type, nhoff, proto); } When net.core.virtio_net_hdr_csum_check is set to 0 and a packet with VIRTIO_NET_HDR_F_NEEDS_CSUM enters virtio_net_hdr_to_skb(), virtio_net_l3_start() still runs pskb_may_pull() and __vlan_get_protocol() to compute nhoff and proto, which virtio_net_csum_partial_ok_at_offset() then immediately discards. Furthermore, if virtio_net_l3_start() returns false (for example, when __vlan_get_protocol() returns 0 on VLAN tags exceeding VLAN_MAX_DEPTH, or on a non-ARPHRD_ETHER device where skb_network_offset(skb) is unset or out of bounds), virtio_net_csum_partial_ok() returns false without consulting virtio_net_hdr_csum_check_disabled(skb). virtio_net_hdr_to_skb() then falls through to virtio_net_resolve_csum_partial_mismatch(), dropping GSO packets or completing non-GSO checksums in software despite the sysctl opt-out. Should virtio_net_csum_partial_ok() check virtio_net_hdr_csum_check_disabled(skb) before calling virtio_net_l3_start()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
