Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] net: use-after-free of iph in ip4_parse() after pskb_may_pull() --
commit f0109d51e8b0adc3b479c25e0e4799b7ae07962f Author: Michael S. Tsirkin <[email protected]> net: add ip4_parse() This commit introduces the ip4_parse() helper in include/linux/ip.h to pull and minimally validate an IPv4 header and its options, converting several callers in virtio_net, bridge netfilter, ipv4 input, and netfilter ipv4 reject. > diff --git a/include/linux/ip.h b/include/linux/ip.h > index d11c25f5030a0..7e047d18a61c9 100644 > --- a/include/linux/ip.h > +++ b/include/linux/ip.h > @@ -56,4 +56,41 @@ static inline void iph_set_totlen(struct iphdr *iph, > unsigned int len) > { > iph->tot_len = len <= IP_MAX_MTU ? htons(len) : 0; > } > + > +/** > + * ip4_parse - pull and minimally validate an IPv4 header > + * @skb: buffer to parse > + * @nhoff: offset of the IPv4 header within the linear data > + * @thoff: returns the transport header offset > + * > + * The whole header, options included, is guaranteed linear on > + * return, so the transport header offset is within the linear > + * data. Callers that need to read the transport header must > + * pull further. > + * > + * Return: the validated IPv4 header, or NULL if truncated or > + * malformed. > + */ > +static inline struct iphdr *ip4_parse(struct sk_buff *skb, > + unsigned int nhoff, > + unsigned int *thoff) > +{ > + struct iphdr *iph; > + > + if (!pskb_may_pull(skb, nhoff + sizeof(struct iphdr))) > + return NULL; > + > + iph = (struct iphdr *)(skb->data + nhoff); > + if (iph->version != 4 || iph->ihl < sizeof(struct iphdr) / sizeof(u32)) > + return NULL; > + > + /* Pull the options as well: the transport header offset > + * returned below must be within the linear data. > + */ > + if (!pskb_may_pull(skb, nhoff + iph->ihl * sizeof(u32))) > + return NULL; > + > + *thoff = nhoff + iph->ihl * sizeof(u32); [Severity: Critical] Can this dereference of iph->ihl after the second pskb_may_pull() cause a use-after-free? In include/linux/ip.h:ip4_parse(), iph is initialized to point into skb->data before the second pskb_may_pull() call pulls the IPv4 options. When IPv4 options (ihl > 5) reside in non-linear skb data and either skb_cloned(skb) is true or skb->head lacks tailroom (or when skb_might_realloc() triggers), the call chain: pskb_may_pull() pskb_may_pull_reason() __pskb_pull_tail() pskb_expand_head() allocates a new skb->head buffer, updates skb->data, and frees the old buffer that iph points into. Reading iph->ihl again afterward to compute *thoff accesses freed memory before skb->data + nhoff is recomputed for the return value. Would computing *thoff = nhoff + iph->ihl * sizeof(u32) before calling pskb_may_pull(skb, *thoff) avoid dereferencing the stale iph pointer? > + return (struct iphdr *)(skb->data + nhoff); > +} > #endif /* _LINUX_IP_H */ [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
