Messages in this thread | | | Date | Tue, 15 Aug 2006 11:08:35 +0200 | From | "Jesper Juhl" <> | Subject | Re: [PATCH] ISDN: fix double free bug in isdn_net |
| |
On 15/08/06, David Miller <davem@davemloft.net> wrote: > From: Jesper Juhl <jesper.juhl@gmail.com> > Date: Sat, 12 Aug 2006 22:48:22 +0200 > > > There's double-free bug in > > drivers/isdn/i4l/isdn_net.c::isdn_net_writebuf_skb(). > > > > If isdn_writebuf_skb_stub() doesn't handle the entire skb, then it will > > have freed the skb that was passed to it and when the code then jumps > > to the error label it'll result in a double free of the skb. > > > > The easy way to fix this is to insert an assignment of skb = NULL in the > > 'if' following the call to isdn_writebuf_skb_stub() so that when the code > > at the error label calls dev_kfree_skb(skb); the skb will be NULL and > > nothing will happen since dev_kfree_skb() just does a return if passed a > > NULL. > > > > Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com> > > I can't ascertain that this is exactly true. > > For example, in isdn_writebuf_skb_stub() we have this: > > if (ret > 0) { > ... > if (dev->v110[idx]) { > ... > if (ret == skb->len) > dev_kfree_skb(skb); >
Hmm, perhaps I made a mistake and missed a path. Maybe it would be better to fix if by making isdn_writebuf_skb_stub() always set the skb to NULL when it does free it. That would add a few more assignments but should ensure the right result always. What do you say?
> So if ret > 0 and we're doing V.110, we only free the skb > if isdn_v110_encode put a value at *((int *)nskb->data) > equal to skb->len. > > So in the V.110 case it's not purely the ->writebuf_skb() return > value that gets returned, rather we return that integer that > isdn_v110_encode() put there. > > Therefore I don't think this SKB leak is %100 correct. What did > I miss? > > BTW, this ISDN code is rotting a bit and could use some cleanups if > not a rewrite. :-/ > A rewrite would be beyond me, but I try to clean up bits and pieces...
-- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |