From: Herbert Xu
> Sent: 04 August 2015 08:43
> Brenden Blanco <bbla...@plumgrid.com> wrote:
> >> [ 318.244596] BUG: unable to handle kernel NULL pointer dereference
> >> at 000000000000008e
> >> [ 318.245182] IP: [<ffffffff81455e7c>] __skb_recv_datagram+0xbc/0x5a0
> >
> > Replying to myself, and adding commit interested parties...
> >
> > I went through the git log for the function in question, and
> > positively identified that the following commit introduces the crash:
> >
> > 738ac1e net: Clone skb before setting peeked flag
> >
> > Null dereference is at line 224 of net/core/datagram.c (according to
> > my objdump dis-assembly):
>
> Sorry, brain fart on my part. Please try this patch.
You've introduced a memory leak if skb_clone() fails.
> The commit 738ac1ebb96d02e0d23bc320302a6ea94c612dec ("net: Clone
> skb before setting peeked flag") introduced a use-after-free bug
> in skb_recv_datagram. This is because skb_set_peeked may create
> a new skb and free the existing one. As it stands the caller will
> continue to use the old freed skb.
>
> This patch fixes it by making skb_set_peeked return the new skb
> (or the old one if unchanged).
...
> nskb = skb_clone(skb, GFP_ATOMIC);
> if (!nskb)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
Here the original skb is still allocated.
> - error = skb_set_peeked(skb);
> - if (error)
> + skb = skb_set_peeked(skb);
You've now lost the address of the original skb.
> + error = PTR_ERR(skb);
> + if (IS_ERR(skb))
> goto unlock_err;
David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
