Re: [PATCH NET-NEXT] ipv6: skb_expand_head() adjust skb->truesize incorrectly
flat view
From: Christoph Paasch <hidden>
Date: 2021-08-23 17:25:59
Also in:
lkml
Hello, On Mon, Aug 23, 2021 at 12:56 AM Vasily Averin [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Christoph Paasch reports [1] about incorrect skb->truesize after skb_expand_head() call in ip6_xmit. This happen because skb_set_owner_w() for newly clone skb is called too early, before pskb_expand_head() where truesize is adjusted for (!skb-sk) case. [1] https://lkml.org/lkml/2021/8/20/1082 Reported-by: Christoph Paasch <redacted> Signed-off-by: Vasily Averin <redacted> --- net/core/skbuff.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)diff --git a/net/core/skbuff.c b/net/core/skbuff.c index f931176..508d5c4 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c@@ -1803,6 +1803,8 @@ struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom) struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) { + struct sk_buff *oskb = skb; + struct sk_buff *nskb = NULL; int delta = headroom - skb_headroom(skb); if (WARN_ONCE(delta <= 0,@@ -1811,21 +1813,21 @@ struct sk_buff *skb_expand_head(struct sk_buff *skb, unsigned int headroom) /* 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(nskb, skb->sk); - consume_skb(skb); - } else { - kfree_skb(skb); - } + nskb = skb_clone(skb, GFP_ATOMIC); skb = nskb; } if (skb && - pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) { - kfree_skb(skb); + pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC)) skb = NULL; + + if (!skb) { + kfree_skb(oskb); + if (nskb) + kfree_skb(nskb); + } else if (nskb) { + if (oskb->sk) + skb_set_owner_w(nskb, oskb->sk); + consume_skb(oskb);
sorry, this does not fix the problem. The syzkaller repro still triggers the WARN. When it happens, the skb in ip6_xmit() is not shared as it comes from __tcp_transmit_skb, where it is skb_clone()'d. Christoph
}
return skb;
}
--
1.8.3.1