Date: Wed, 14 Jan 2004 17:43:56 +0900 (JST) From: itojun@itojun.org (Jun-ichiro itojun Hagino) To: bzeeb-lists@lists.zabbadoz.net Cc: ume@freebsd.org Subject: Re: [PATCH] IPSec fixes Message-ID: <20040114084356.C894EA0@coconut.itojun.org> In-Reply-To: Your message of "Wed, 14 Jan 2004 05:48:39 %2B0000 (UTC)" <Pine.BSF.4.53.0401140532090.30149@e0-0.zab2.int.zabbadoz.net> References: <Pine.BSF.4.53.0401140532090.30149@e0-0.zab2.int.zabbadoz.net>
next in thread | previous in thread | raw e-mail | index | archive | help
> > as for key_sp_unlink(), i don't think the patch is correct. > > even if you do not call key_sp_unlink() in key_spdflush(), spd entries > > will get unlink'ed in key_timehandler(). therefore the end result > > will be the same. > > No ! calling key_sp_unlink() from key_spdflush() will result in an > _extra_ call of key_freesp() and thus refcnt will be decremented > though it shouldn't. > This will result in a refcnt being 0 too early and with valid > pointers to that secpolicy and will further lead to "Memory accessed > and/or modified after free" situations somewhen after the first and > all successive flushes of the SPD. > Each part of the code checks for the state == .._DEAD when getting an > sp from sptree so the comment above key_spdflush() is correct. Only > mark the sp as dead. > > Hope this explains the problem a bit better. the refcnt decremented in key_sp_unlink() is for the link from sptree[]. i guess this is the proper fix. does it fix your situation? itojun Index: key.c =================================================================== RCS file: /cvsroot/kame/kame/kame/sys/netkey/key.c,v retrieving revision 1.324 diff -u -r1.324 key.c --- key.c 14 Jan 2004 04:10:24 -0000 1.324 +++ key.c 14 Jan 2004 08:43:18 -0000 @@ -2092,9 +2092,9 @@ newsp->lifetime = lft ? lft->sadb_lifetime_addtime : 0; newsp->validtime = lft ? lft->sadb_lifetime_usetime : 0; - newsp->refcnt = 1; /* do not reclaim until I say I do */ newsp->state = IPSEC_SPSTATE_ALIVE; LIST_INSERT_TAIL(&sptree[newsp->dir], newsp, secpolicy, chain); + newsp->refcnt++; /* delete the entry in spacqtree */ if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE &&
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040114084356.C894EA0>