Re: [PATCH IPV6 v3 1/1] ipv6: allocate enough headroom in ip6_finish_output2()
From: Vasily Averin <hidden>
Date: 2021-07-13 07:46:24
Also in:
lkml
I've found 2 problems in this patch, and I'm going to resend new patch version soon. On 7/12/21 9:45 AM, Vasily Averin wrote:
quoted hunk ↗ jump to hunk
index ff4f9eb..0efcb9b 100644--- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c@@ -60,10 +60,38 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff * { struct dst_entry *dst = skb_dst(skb); struct net_device *dev = dst->dev; + unsigned int hh_len = LL_RESERVED_SPACE(dev); + int delta = hh_len - skb_headroom(skb); const struct in6_addr *nexthop; struct neighbour *neigh; int ret; + /* Be paranoid, rather than too clever. */ + if (unlikely(delta > 0) && dev->header_ops) { + /* pskb_expand_head() might crash, if skb is shared */ + if (skb_shared(skb)) { + struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); + + if (likely(nskb)) { + if (skb->sk) + skb_set_owner_w(skb, skb->sk);
need to assign sk not to skb but to nskb
+ consume_skb(skb);
+ } else {
+ kfree_skb(skb);It is quite strange to call consume_skb() on one case and kfree_skb() in another one. We know that original skb was shared so we should not call kfree_skb here. Btw I've noticed similar problem in few other cases: in pptp_xmit, pvc_xmit, ip_vs_prepare_tunneled_skb they call consume_skb() in case of success and kfree_skb on error path. It looks like potential bug for me.
+ }
+ skb = nskb;
+ }
+ if (skb &&
+ pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) {
+ kfree_skb(skb);
+ skb = NULL;
+ }
+ if (!skb) {
+ IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTDISCARDS);
+ return -ENOMEM;
+ }
+ }
+
if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) {
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));