Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] net: use-after-free read of iph->ihl after pskb_may_pull() in ip4_parse() -- > diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h > --- a/include/linux/virtio_net.h > +++ b/include/linux/virtio_net.h > @@ -638,7 +638,6 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, > * We know VIRTIO_NET_HDR_F_NEEDS_CSUM was set so > __virtio_net_hdr_to_skb > * set CHECKSUM_PARTIAL. > */ > - inner_proto = inner_isv6 ? htons(ETH_P_IPV6) : htons(ETH_P_IP); > if (virtio_net_csum_partial_ok_at_offset(skb, gso_inner_type, inner_nh, > inner_proto)) > return 0; [Severity: High] This isn't introduced by this patch, but since inner_proto is what steers virtio_net_csum_partial_ok_at_offset() into the IPv4 parser, it's worth raising here. virtio_net_csum_partial_ok_at_offset() calls virtio_net_l4_start(), which for ETH_P_IP calls ip4_parse(). In ip4_parse() the fixed header is pulled, iph is set to skb->data + nhoff, and then a second pskb_may_pull(skb, nhoff + iph->ihl * sizeof(u32)) pulls in the options. That second pull can go through __pskb_pull_tail() and pskb_expand_head() when the options sit in paged frags, the skb is cloned, or CONFIG_FAIL_SKB_REALLOC fires, and that frees the old skb->head. The next line then computes *thoff from iph->ihl through the stale pointer, before the header is reloaded from skb->data. That is a use-after-free read that a guest, or a tun/tap/af_packet user, can trigger with an IPv4 header whose options straddle the linear area. If the freed buffer has been reused, we can also end up with a bogus *thoff that is then checked against csum_start. Since ip4_parse() is shared with ip_rcv_core(), br_validate_ipv4() and nf_reject_iphdr_validate(), plain IPv4 ingress hits it too. Reading ihl into a local before the second pskb_may_pull(), or reloading iph from skb->data right after it, should be enough. Do you want to fold a fix for ip4_parse() into this series, or send it separately to net with a Fixes: tag for the commit that added ip4_parse()? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.v5.git.mst%40redhat.com
