On Mon, Apr 23, 2007 at 08:44:16AM +0200, Jarek Poplawski wrote: > On Fri, Apr 20, 2007 at 04:35:15PM -0700, David Miller wrote: > > From: Jarek Poplawski <[EMAIL PROTECTED]> > > Date: Mon, 12 Mar 2007 11:24:03 +0100 > > > > > > the ipcomp handler is xfrm6_rcv(), which calls xfrm6_rcv_spi(), which > > > > contrary > > > > to all other handlers returns -1 instead of 0 after calling kfree_skb() > > > > on the > > > > skb. Changing the return value to 0 in xfrm6_input.c:xfrm6_rcv_spi() > > > > fixes the > > > > problem. > > > > But I got no clue at all if this would be a correct fix > > > > > > I think your diagnose is correct (all "return -1" should be > > > changed to "return 0" in xfrm6_input.c). > > I've corrected this, yet: > > "Sorry! Of course should be: > I think your diagnose is correct (all "return -1" should be > changed to "return 0" in xfrm6_rcv_spi())." > > It's just like Eric diagnosed: > > xfrm6_rcv() calls tunnel6_rcv(), which calls handlers->handler() > and if handler() returns anything but 0, skb is kfreed. But > handler: xfrm6_tunnel_rcv() calls xfrm6_rcv_spi() and returns its > return without changing, which is only 1 and -1. It seems, in > every -1 case skb is kfreed by xfrm6_rcv_spi() or by functions > called by it, probably meaning skb was handled (delivered or > kfreed). The only path where skb is not kfreed returns 1. > > tunnel6_rcv() treats both returns the same way - so some skbs > are kfreed 2 times.
OK, now I see this place is really "special" and there is more than this. The same handler is used for 2 things, which expect different error codes for similar things. My proposal is: maybe Eric could change this in xfrm6_tunnel_rcv() from xfrm6_tunnel.c e.g. like this: return xfrm6_rcv_spi(skb, spi) > 0 ? : 0; and, if no errors in testing, he could resubmit this patch? Regards, Jarek P. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
