[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1CB727D0@AcuExch.aculab.com>
Date: Tue, 4 Aug 2015 09:24:44 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Herbert Xu' <herbert@...dor.apana.org.au>
CC: Brenden Blanco <bblanco@...mgrid.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"khlebnikov@...dex-team.ru" <khlebnikov@...dex-team.ru>
Subject: RE: Fix skb_set_peeked use-after-free bug
From: Herbert Xu
> Sent: 04 August 2015 10:21
> On Tue, Aug 04, 2015 at 09:15:13AM +0000, David Laight wrote:
> >
> > You've introduced a memory leak if skb_clone() fails.
>
> No I have not.
>
> > > 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.
>
> It doesn't matter because we will take the error path and return
> the ENOMEM error. We must not free the skb as it's still on the
> recv queue.
In that case, what happens to the receive queue when skb_clone()
takes a copy of the skb - freeing the original one?
David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists