Re: pskb_expand_head: skb_shared BUG

[Chris Dunlop <chris@onthe.net.au>]

On Mon, Mar 02, 2015 at 11:45:11AM +1100, Chris Dunlop wrote:
> Heads up...
> 
> We've hit this BUG() in v3.10.70, v3.14.27 and v3.18.7:
> 
> net/core/skbuff.c:
> 1027 int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
> 1028                      gfp_t gfp_mask)
> 1029 {
> 1030         int i;
> 1031         u8 *data;
> 1032         int size = nhead + skb_end_offset(skb) + ntail;
> 1033         long off;
> 1034 
> 1035         BUG_ON(nhead < 0);
> 1036 
> 1037         if (skb_shared(skb))
> 1038                 BUG();             <<< BOOM!!!
> 
> This appears to be a regression in the 3.10.x stable series:
> we've been running for 11 months on v3.10.33 without problem, we
> upgraded to v3.14.27 and hit the BUG(), than again on upgrading
> to v3.18.7, then again after downgrading to v3.10.70. 

I'm guessing this is the cause:

commit 522ad79b7fd042f7f735bf1826f2e5b564d8f015
Author: Jiri Benc <jbenc@redhat.com>
Date:   Thu Aug 21 21:33:44 2014 +0200

    openvswitch: fix panic with multiple vlan headers

 static int make_writable(struct sk_buff *skb, int write_len)
 {
+       if (!pskb_may_pull(skb, write_len))
+               return -ENOMEM;
+
        if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
                return 0;

...which takes us through:

static inline int pskb_may_pull(struct sk_buff *skb, unsigned int len)
{
        if (likely(len <= skb_headlen(skb)))
                return 1;
        if (unlikely(len > skb->len))
                return 0;
        return __pskb_pull_tail(skb, len - skb_headlen(skb)) != NULL;
}

unsigned char *__pskb_pull_tail(struct sk_buff *skb, int delta)
{
        int eat = (skb->tail + delta) - skb->end;
        ...
        if (eat > 0 || skb_cloned(skb)) {
                if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
                                GFP_ATOMIC))
                        return NULL;
        }
        ...
}

pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0, GFP_ATOMIC)
{
        ...
        if (skb_shared(skb))
                BUG();                          <<< BOOM!!!
        ...
}


Cheers,

Chris