From 10c711c5698164de1d3a6c5ea2aaafd74177a217 Mon Sep 17 00:00:00 2001 From: jonathan Date: Wed, 26 May 2004 23:16:25 +0000 Subject: [PATCH] Fix bugs in SPD refcounts due to PCBpolicy cache, by backporting the KAME sys/netkey/key.c rev 1.119 ke_sp_unlink()/key_sp_dead() logic. I have been running a similar version for about 10 days now, and it fixes the PCB-cache refcount problems for me. Checked in as a candidate for pullup to the 2.0 branch. --- sys/netipsec/key.c | 71 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c index 54025d663276..9b88c3bdbbf9 100644 --- a/sys/netipsec/key.c +++ b/sys/netipsec/key.c @@ -1,4 +1,4 @@ -/* $NetBSD: key.c,v 1.17 2004/05/26 22:14:18 jonathan Exp $ */ +/* $NetBSD: key.c,v 1.18 2004/05/26 23:16:25 jonathan Exp $ */ /* $FreeBSD: src/sys/netipsec/key.c,v 1.3.2.3 2004/02/14 22:23:23 bms Exp $ */ /* $KAME: key.c,v 1.191 2001/06/27 10:46:49 sakane Exp $ */ @@ -32,7 +32,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.17 2004/05/26 22:14:18 jonathan Exp $"); +__KERNEL_RCSID(0, "$NetBSD: key.c,v 1.18 2004/05/26 23:16:25 jonathan Exp $"); /* * This code is referd to RFC 2367 @@ -489,6 +489,9 @@ static const char *key_getfqdn __P((void)); static const char *key_getuserfqdn __P((void)); #endif static void key_sa_chgstate __P((struct secasvar *, u_int8_t)); +static __inline void key_sp_dead __P((struct secpolicy *)); +static void key_sp_unlink __P((struct secpolicy *sp)); + static struct mbuf *key_alloc_mbuf __P((int)); struct callout key_timehandler_ch; @@ -514,6 +517,28 @@ struct callout key_timehandler_ch; (p)->refcnt--; \ } while (0) + +static __inline void +key_sp_dead(struct secpolicy *sp) +{ + + /* mark the SP dead */ + sp->state = IPSEC_SPSTATE_DEAD; +} + +static void +key_sp_unlink(struct secpolicy *sp) +{ + + /* remove from SP index */ + if (__LIST_CHAINED(sp)) { + LIST_REMOVE(sp, chain); + /* Release refcount held just for being on chain */ + KEY_FREESP(&sp); + } +} + + /* * Return 0 when there are known to be no SP's for the specified * direction. Otherwise return 1. This is used by IPsec code @@ -1188,16 +1213,13 @@ key_delsp(struct secpolicy *sp) IPSEC_ASSERT(sp != NULL, ("key_delsp: null sp")); - sp->state = IPSEC_SPSTATE_DEAD; + key_sp_dead(sp); IPSEC_ASSERT(sp->refcnt == 0, ("key_delsp: SP with references deleted (refcnt %u)", sp->refcnt)); s = splsoftnet(); /*called from softclock()*/ - /* remove from SP index */ - if (__LIST_CHAINED(sp)) - LIST_REMOVE(sp, chain); { struct ipsecrequest *isr = sp->req, *nextisr; @@ -1803,8 +1825,10 @@ key_spdadd(so, m, mhp) newsp = key_getsp(&spidx); if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE) { if (newsp) { - newsp->state = IPSEC_SPSTATE_DEAD; + key_sp_dead(newsp); + key_sp_unlink(newsp); /* XXX jrs ordering */ KEY_FREESP(&newsp); + newsp = NULL; } } else { if (newsp != NULL) { @@ -2042,8 +2066,9 @@ key_spddelete(so, m, mhp) /* save policy id to buffer to be returned. */ xpl0->sadb_x_policy_id = sp->id; - sp->state = IPSEC_SPSTATE_DEAD; - KEY_FREESP(&sp); + key_sp_dead(sp); + key_sp_unlink(sp); /* XXX jrs ordering */ + KEY_FREESP(&sp); /* ref gained by key_getspbyid */ #if defined(__NetBSD__) /* Invalidate all cached SPD pointers in the PCBs. */ @@ -2111,8 +2136,10 @@ key_spddelete2(so, m, mhp) key_senderror(so, m, EINVAL); } - sp->state = IPSEC_SPSTATE_DEAD; - KEY_FREESP(&sp); + key_sp_dead(sp); + key_sp_unlink(sp); /* XXX jrs ordering */ + KEY_FREESP(&sp); /* ref gained by key_getsp */ + sp = NULL; #if defined(__NetBSD__) /* Invalidate all cached SPD pointers in the PCBs. */ @@ -2325,8 +2352,18 @@ key_spdflush(so, m, mhp) return key_senderror(so, m, EINVAL); for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { - LIST_FOREACH(sp, &sptree[dir], chain) { - sp->state = IPSEC_SPSTATE_DEAD; + struct secpolicy * nextsp; + for (sp = LIST_FIRST(&sptree[dir]); + sp != NULL; + sp = nextsp) { + + nextsp = LIST_NEXT(sp, chain); + if (sp->state == IPSEC_SPSTATE_DEAD) + continue; + key_sp_dead(sp); + key_sp_unlink(sp); + /* 'sp' dead; continue transfers to 'sp = nextsp' */ + continue; } } @@ -4119,7 +4156,11 @@ key_timehandler(void* arg) nextsp = LIST_NEXT(sp, chain); if (sp->state == IPSEC_SPSTATE_DEAD) { - KEY_FREESP(&sp); + key_sp_unlink(sp); /*XXX*/ + + /* 'sp' dead; continue transfers to + * 'sp = nextsp' + */ continue; } @@ -4129,7 +4170,7 @@ key_timehandler(void* arg) /* the deletion will occur next time */ if ((sp->lifetime && now - sp->created > sp->lifetime) || (sp->validtime && now - sp->lastused > sp->validtime)) { - sp->state = IPSEC_SPSTATE_DEAD; + key_sp_dead(sp); key_spdexpire(sp); continue; }