commit 9fb07e928a7c503a38609eb10767c80595ff7901 Author: Andrey V. Elsukov Date: Wed Dec 3 06:07:42 2014 +0300 Refine SPD locking. * changes in struct secpolicy: - use TAILQ to organize the chain; - remove unused mutex; - remove scangen field; - remove state field. It has only two states DEAD and ALIVE. Now we just remove entries with DEAD state. The chain contains only ALIVE entries. * move default secpolicy initialization to key.c; * convert sptree_lock to rmlock. Convert most of walktrough loops to use RLOCK and use WLOCK when we are changing the V_sptree chain. * convert SP_ADDREF/SP_DELREF macros to proper use of refcount(9). Now we don't keep DEAD entries in the chain, and free them when refcount_release() releases last reference. * remove key_delsp and _key_delsp functions, merge them to the _key_freesp() and only use KEY_FREESP() macro to release reference. * fix SPACQ lock leak in the key_spdacquire(); * ANSIfy some functions and fix style(9). diff --git a/sys/netipsec/ipsec.c b/sys/netipsec/ipsec.c index 419a4d5..991ce4d 100644 --- a/sys/netipsec/ipsec.c +++ b/sys/netipsec/ipsec.c @@ -830,15 +830,12 @@ ipsec_init_policy(struct socket *so, struct inpcbpolicy **pcb_sp) ipsec_delpcbpolicy(new); return (ENOBUFS); } - new->sp_in->state = IPSEC_SPSTATE_ALIVE; new->sp_in->policy = IPSEC_POLICY_ENTRUST; - if ((new->sp_out = KEY_NEWSP()) == NULL) { KEY_FREESP(&new->sp_in); ipsec_delpcbpolicy(new); return (ENOBUFS); } - new->sp_out->state = IPSEC_SPSTATE_ALIVE; new->sp_out->policy = IPSEC_POLICY_ENTRUST; *pcb_sp = new; @@ -929,7 +926,6 @@ ipsec_deepcopy_policy(struct secpolicy *src) } dst->req = newchain; - dst->state = src->state; dst->policy = src->policy; /* Do not touch the refcnt fields. */ @@ -981,8 +977,6 @@ ipsec_set_policy_internal(struct secpolicy **pcb_sp, int optname, if ((newsp = key_msg2sp(xpl, len, &error)) == NULL) return (error); - newsp->state = IPSEC_SPSTATE_ALIVE; - /* Clear old SP and set new SP. */ KEY_FREESP(pcb_sp); *pcb_sp = newsp; @@ -1699,17 +1693,6 @@ ipsec_dumpmbuf(struct mbuf *m) printf("---\n"); } -static void -ipsec_init(const void *unused __unused) -{ - - SECPOLICY_LOCK_INIT(&V_ip4_def_policy); - V_ip4_def_policy.refcnt = 1; /* NB: disallow free. */ -} -VNET_SYSINIT(ipsec_init, SI_SUB_PROTO_DOMAININIT, SI_ORDER_ANY, ipsec_init, - NULL); - - /* XXX This stuff doesn't belong here... */ static struct xformsw* xforms = NULL; diff --git a/sys/netipsec/ipsec.h b/sys/netipsec/ipsec.h index 694eb08..1f4b42f 100644 --- a/sys/netipsec/ipsec.h +++ b/sys/netipsec/ipsec.h @@ -81,21 +81,15 @@ struct secpolicyindex { /* Security Policy Data Base */ struct secpolicy { - LIST_ENTRY(secpolicy) chain; - struct mtx lock; + TAILQ_ENTRY(secpolicy) chain; - u_int refcnt; /* reference count */ struct secpolicyindex spidx; /* selector */ - u_int32_t id; /* It's unique number on the system. */ - u_int state; /* 0: dead, others: alive */ -#define IPSEC_SPSTATE_DEAD 0 -#define IPSEC_SPSTATE_ALIVE 1 - u_int policy; /* policy_type per pfkeyv2.h */ - u_int16_t scangen; /* scan generation # */ struct ipsecrequest *req; /* pointer to the ipsec request tree, */ /* if policy == IPSEC else this value == NULL.*/ - + u_int refcnt; /* reference count */ + u_int policy; /* policy_type per pfkeyv2.h */ + u_int32_t id; /* It's unique number on the system. */ /* * lifetime handler. * the policy can be used without limitiation if both lifetime and @@ -109,13 +103,6 @@ struct secpolicy { long validtime; /* duration this policy is valid without use */ }; -#define SECPOLICY_LOCK_INIT(_sp) \ - mtx_init(&(_sp)->lock, "ipsec policy", NULL, MTX_DEF) -#define SECPOLICY_LOCK(_sp) mtx_lock(&(_sp)->lock) -#define SECPOLICY_UNLOCK(_sp) mtx_unlock(&(_sp)->lock) -#define SECPOLICY_LOCK_DESTROY(_sp) mtx_destroy(&(_sp)->lock) -#define SECPOLICY_LOCK_ASSERT(_sp) mtx_assert(&(_sp)->lock, MA_OWNED) - /* Request for IPsec */ struct ipsecrequest { struct ipsecrequest *next; diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c index 54d51ff..bef6152 100644 --- a/sys/netipsec/key.c +++ b/sys/netipsec/key.c @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -141,16 +142,20 @@ static VNET_DEFINE(u_int32_t, acq_seq) = 0; #define V_acq_seq VNET(acq_seq) /* SPD */ -static VNET_DEFINE(LIST_HEAD(_sptree, secpolicy), sptree[IPSEC_DIR_MAX]); +static VNET_DEFINE(TAILQ_HEAD(_sptree, secpolicy), sptree[IPSEC_DIR_MAX]); +static struct rmlock sptree_lock; #define V_sptree VNET(sptree) -static struct mtx sptree_lock; -#define SPTREE_LOCK_INIT() \ - mtx_init(&sptree_lock, "sptree", \ - "fast ipsec security policy database", MTX_DEF) -#define SPTREE_LOCK_DESTROY() mtx_destroy(&sptree_lock) -#define SPTREE_LOCK() mtx_lock(&sptree_lock) -#define SPTREE_UNLOCK() mtx_unlock(&sptree_lock) -#define SPTREE_LOCK_ASSERT() mtx_assert(&sptree_lock, MA_OWNED) + +#define SPTREE_LOCK_INIT() rm_init(&sptree_lock, "sptree") +#define SPTREE_LOCK_DESTROY() rm_destroy(&sptree_lock) +#define SPTREE_RLOCK_TRACKER struct rm_priotracker sptree_tracker +#define SPTREE_RLOCK() rm_rlock(&sptree_lock, &sptree_tracker) +#define SPTREE_RUNLOCK() rm_runlock(&sptree_lock, &sptree_tracker) +#define SPTREE_RLOCK_ASSERT() rm_assert(&sptree_lock, RA_RLOCKED) +#define SPTREE_WLOCK() rm_wlock(&sptree_lock) +#define SPTREE_WUNLOCK() rm_wunlock(&sptree_lock) +#define SPTREE_WLOCK_ASSERT() rm_assert(&sptree_lock, RA_WLOCKED) +#define SPTREE_UNLOCK_ASSERT() rm_assert(&sptree_lock, RA_UNLOCKED) static VNET_DEFINE(LIST_HEAD(_sahtree, secashead), sahtree); /* SAD */ #define V_sahtree VNET(sahtree) @@ -415,10 +420,9 @@ static struct callout key_timer; static struct secasvar *key_allocsa_policy(const struct secasindex *); static void key_freesp_so(struct secpolicy **); +static void key_unlink(struct secpolicy *); static struct secasvar *key_do_allocsa_policy(struct secashead *, u_int); -static void key_delsp(struct secpolicy *); static struct secpolicy *key_getsp(struct secpolicyindex *); -static void _key_delsp(struct secpolicy *sp); static struct secpolicy *key_getspbyid(u_int32_t); static u_int32_t key_newreqid(void); static struct mbuf *key_gather_mbuf(struct mbuf *, @@ -574,16 +578,26 @@ sa_delref(struct secasvar *sav) IPSEC_ASSERT(sav->refcnt > 0, ("SA refcnt underflow")); return (refcount_release(&sav->refcnt)); } +static __inline void +sp_initref(struct secpolicy *sp) +{ -#define SP_ADDREF(p) do { \ - (p)->refcnt++; \ - IPSEC_ASSERT((p)->refcnt != 0, ("SP refcnt overflow")); \ -} while (0) -#define SP_DELREF(p) do { \ - IPSEC_ASSERT((p)->refcnt > 0, ("SP refcnt underflow")); \ - (p)->refcnt--; \ -} while (0) - + refcount_init(&sp->refcnt, 1); +} +static __inline void +sp_addref(struct secpolicy *sp) +{ + + refcount_acquire(&sp->refcnt); + IPSEC_ASSERT(sp->refcnt != 0, ("SP refcnt overflow")); +} +static __inline int +sp_delref(struct secpolicy *sp) +{ + + IPSEC_ASSERT(sp->refcnt > 0, ("SP refcnt underflow")); + return (refcount_release(&sp->refcnt)); +} /* * Update the refcnt while holding the SPTREE lock. @@ -591,11 +605,20 @@ sa_delref(struct secasvar *sav) void key_addref(struct secpolicy *sp) { - SPTREE_LOCK(); - SP_ADDREF(sp); - SPTREE_UNLOCK(); + + sp_addref(sp); } +static void +def_policy_init(const void *unused __unused) +{ + + bzero(&V_ip4_def_policy, sizeof(struct secpolicy)); + sp_initref(&V_ip4_def_policy); +} +VNET_SYSINIT(def_policy_init, SI_SUB_PROTO_DOMAININIT, SI_ORDER_ANY, + def_policy_init, NULL); + /* * Return 0 when there are known to be no SP's for the specified * direction. Otherwise return 1. This is used by IPsec code @@ -606,7 +629,7 @@ key_havesp(u_int dir) { return (dir == IPSEC_DIR_INBOUND || dir == IPSEC_DIR_OUTBOUND ? - LIST_FIRST(&V_sptree[dir]) != NULL : 1); + TAILQ_FIRST(&V_sptree[dir]) != NULL : 1); } /* %%% IPsec policy management */ @@ -620,6 +643,7 @@ struct secpolicy * key_allocsp(struct secpolicyindex *spidx, u_int dir, const char* where, int tag) { + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; IPSEC_ASSERT(spidx != NULL, ("null spidx")); @@ -634,14 +658,11 @@ key_allocsp(struct secpolicyindex *spidx, u_int dir, const char* where, printf("*** objects\n"); kdebug_secpolicyindex(spidx)); - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[dir], chain) { + SPTREE_RLOCK(); + TAILQ_FOREACH(sp, &V_sptree[dir], chain) { KEYDEBUG(KEYDEBUG_IPSEC_DATA, printf("*** in SPD\n"); kdebug_secpolicyindex(&sp->spidx)); - - if (sp->state == IPSEC_SPSTATE_DEAD) - continue; if (key_cmpspidx_withmask(&sp->spidx, spidx)) goto found; } @@ -653,9 +674,9 @@ found: /* found a SPD entry */ sp->lastused = time_second; - SP_ADDREF(sp); + sp_addref(sp); } - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); KEYDEBUG(KEYDEBUG_IPSEC_STAMP, printf("DP %s return SP:%p (ID=%u) refcnt %u\n", __func__, @@ -673,6 +694,7 @@ struct secpolicy * key_allocsp2(u_int32_t spi, union sockaddr_union *dst, u_int8_t proto, u_int dir, const char* where, int tag) { + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; IPSEC_ASSERT(dst != NULL, ("null dst")); @@ -688,14 +710,12 @@ key_allocsp2(u_int32_t spi, union sockaddr_union *dst, u_int8_t proto, printf("spi %u proto %u dir %u\n", spi, proto, dir); kdebug_sockaddr(&dst->sa)); - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[dir], chain) { + SPTREE_RLOCK(); + TAILQ_FOREACH(sp, &V_sptree[dir], chain) { KEYDEBUG(KEYDEBUG_IPSEC_DATA, printf("*** in SPD\n"); kdebug_secpolicyindex(&sp->spidx)); - if (sp->state == IPSEC_SPSTATE_DEAD) - continue; /* compare simple values, then dst address */ if (sp->spidx.ul_proto != proto) continue; @@ -713,9 +733,9 @@ found: /* found a SPD entry */ sp->lastused = time_second; - SP_ADDREF(sp); + sp_addref(sp); } - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); KEYDEBUG(KEYDEBUG_IPSEC_STAMP, printf("DP %s return SP:%p (ID=%u) refcnt %u\n", __func__, @@ -1169,31 +1189,6 @@ done: /* * Must be called after calling key_allocsp(). - * For both the packet without socket and key_freeso(). - */ -void -_key_freesp(struct secpolicy **spp, const char* where, int tag) -{ - struct secpolicy *sp = *spp; - - IPSEC_ASSERT(sp != NULL, ("null sp")); - - SPTREE_LOCK(); - SP_DELREF(sp); - - KEYDEBUG(KEYDEBUG_IPSEC_STAMP, - printf("DP %s SP:%p (ID=%u) from %s:%u; refcnt now %u\n", - __func__, sp, sp->id, where, tag, sp->refcnt)); - - if (sp->refcnt == 0) { - *spp = NULL; - key_delsp(sp); - } - SPTREE_UNLOCK(); -} - -/* - * Must be called after calling key_allocsp(). * For the packet with socket. */ void @@ -1278,35 +1273,46 @@ key_freesav(struct secasvar **psav, const char* where, int tag) /* %%% SPD management */ /* - * free security policy entry. + * Must be called after calling key_allocsp(). + * For both the packet without socket and key_freeso(). */ -static void -key_delsp(struct secpolicy *sp) +void +_key_freesp(struct secpolicy **spp, const char* where, int tag) { struct ipsecrequest *isr, *nextisr; + struct secpolicy *sp = *spp; IPSEC_ASSERT(sp != NULL, ("null sp")); - SPTREE_LOCK_ASSERT(); - - sp->state = IPSEC_SPSTATE_DEAD; - - IPSEC_ASSERT(sp->refcnt == 0, - ("SP with references deleted (refcnt %u)", sp->refcnt)); - - /* remove from SP index */ - if (__LIST_CHAINED(sp)) - LIST_REMOVE(sp, chain); - + KEYDEBUG(KEYDEBUG_IPSEC_STAMP, + printf("DP %s SP:%p (ID=%u) from %s:%u; refcnt %u\n", + __func__, sp, sp->id, where, tag, sp->refcnt)); + if (sp_delref(sp) == 0) + return; + *spp = NULL; for (isr = sp->req; isr != NULL; isr = nextisr) { if (isr->sav != NULL) { KEY_FREESAV(&isr->sav); isr->sav = NULL; } - nextisr = isr->next; ipsec_delisr(isr); } - _key_delsp(sp); + free(sp, M_IPSEC_SP); +} + +static void +key_unlink(struct secpolicy *sp) +{ + + IPSEC_ASSERT(sp != NULL, ("null sp")); + IPSEC_ASSERT(sp->spidx.dir == IPSEC_DIR_INBOUND || + sp->spidx.dir == IPSEC_DIR_OUTBOUND, + ("invalid direction %u", sp->spidx.dir)); + SPTREE_UNLOCK_ASSERT(); + + SPTREE_WLOCK(); + TAILQ_REMOVE(&V_sptree[sp->spidx.dir], sp, chain); + SPTREE_WUNLOCK(); } /* @@ -1317,20 +1323,19 @@ key_delsp(struct secpolicy *sp) static struct secpolicy * key_getsp(struct secpolicyindex *spidx) { + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; IPSEC_ASSERT(spidx != NULL, ("null spidx")); - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[spidx->dir], chain) { - if (sp->state == IPSEC_SPSTATE_DEAD) - continue; + SPTREE_RLOCK(); + TAILQ_FOREACH(sp, &V_sptree[spidx->dir], chain) { if (key_cmpspidx_exactly(spidx, &sp->spidx)) { - SP_ADDREF(sp); + sp_addref(sp); break; } } - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); return sp; } @@ -1343,28 +1348,25 @@ key_getsp(struct secpolicyindex *spidx) static struct secpolicy * key_getspbyid(u_int32_t id) { + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[IPSEC_DIR_INBOUND], chain) { - if (sp->state == IPSEC_SPSTATE_DEAD) - continue; + SPTREE_RLOCK(); + TAILQ_FOREACH(sp, &V_sptree[IPSEC_DIR_INBOUND], chain) { if (sp->id == id) { - SP_ADDREF(sp); + sp_addref(sp); goto done; } } - LIST_FOREACH(sp, &V_sptree[IPSEC_DIR_OUTBOUND], chain) { - if (sp->state == IPSEC_SPSTATE_DEAD) - continue; + TAILQ_FOREACH(sp, &V_sptree[IPSEC_DIR_OUTBOUND], chain) { if (sp->id == id) { - SP_ADDREF(sp); + sp_addref(sp); goto done; } } done: - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); return sp; } @@ -1376,25 +1378,14 @@ key_newsp(const char* where, int tag) newsp = (struct secpolicy *) malloc(sizeof(struct secpolicy), M_IPSEC_SP, M_NOWAIT|M_ZERO); - if (newsp) { - SECPOLICY_LOCK_INIT(newsp); - newsp->refcnt = 1; - newsp->req = NULL; - } - + if (newsp) + sp_initref(newsp); KEYDEBUG(KEYDEBUG_IPSEC_STAMP, printf("DP %s from %s:%u return SP:%p\n", __func__, where, tag, newsp)); return newsp; } -static void -_key_delsp(struct secpolicy *sp) -{ - SECPOLICY_LOCK_DESTROY(sp); - free(sp, M_IPSEC_SP); -} - /* * create secpolicy structure from sadb_x_policy structure. * NOTE: `state', `secpolicyindex' in secpolicy structure are not set, @@ -1874,9 +1865,7 @@ key_spdadd(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) newsp = key_getsp(&spidx); if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE) { if (newsp) { - SPTREE_LOCK(); - newsp->state = IPSEC_SPSTATE_DEAD; - SPTREE_UNLOCK(); + key_unlink(newsp); KEY_FREESP(&newsp); } } else { @@ -1888,13 +1877,17 @@ key_spdadd(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) } } + /* + * XXX: there is race between key_getsp and key_msg2sp. + */ + /* allocation new SP entry */ if ((newsp = key_msg2sp(xpl0, PFKEY_EXTLEN(xpl0), &error)) == NULL) { return key_senderror(so, m, error); } - if ((newsp->id = key_getnewspid()) == 0) { - _key_delsp(newsp); + if (newsp->id == 0 && (newsp->id = key_getnewspid()) == 0) { + KEY_FREESP(&newsp); return key_senderror(so, m, ENOBUFS); } @@ -1910,31 +1903,30 @@ key_spdadd(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) /* sanity check on addr pair */ if (((struct sockaddr *)(src0 + 1))->sa_family != ((struct sockaddr *)(dst0+ 1))->sa_family) { - _key_delsp(newsp); + KEY_FREESP(&newsp); return key_senderror(so, m, EINVAL); } if (((struct sockaddr *)(src0 + 1))->sa_len != ((struct sockaddr *)(dst0+ 1))->sa_len) { - _key_delsp(newsp); + KEY_FREESP(&newsp); return key_senderror(so, m, EINVAL); } -#if 1 - if (newsp->req && newsp->req->saidx.src.sa.sa_family && newsp->req->saidx.dst.sa.sa_family) { - if (newsp->req->saidx.src.sa.sa_family != newsp->req->saidx.dst.sa.sa_family) { - _key_delsp(newsp); + if (newsp->req && newsp->req->saidx.src.sa.sa_family && + newsp->req->saidx.dst.sa.sa_family) { + if (newsp->req->saidx.src.sa.sa_family != + newsp->req->saidx.dst.sa.sa_family) { + KEY_FREESP(&newsp); return key_senderror(so, m, EINVAL); } } -#endif newsp->created = time_second; newsp->lastused = newsp->created; 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(&V_sptree[newsp->spidx.dir], newsp, secpolicy, chain); + SPTREE_WLOCK(); + TAILQ_INSERT_TAIL(&V_sptree[newsp->spidx.dir], newsp, chain); + SPTREE_WUNLOCK(); /* delete the entry in spacqtree */ if (mhp->msg->sadb_msg_type == SADB_X_SPDUPDATE) { @@ -2108,10 +2100,7 @@ key_spddelete(struct socket *so, struct mbuf *m, /* save policy id to buffer to be returned. */ xpl0->sadb_x_policy_id = sp->id; - - SPTREE_LOCK(); - sp->state = IPSEC_SPSTATE_DEAD; - SPTREE_UNLOCK(); + key_unlink(sp); KEY_FREESP(&sp); { @@ -2175,10 +2164,7 @@ key_spddelete2(struct socket *so, struct mbuf *m, ipseclog((LOG_DEBUG, "%s: no SP found id:%u.\n", __func__, id)); return key_senderror(so, m, EINVAL); } - - SPTREE_LOCK(); - sp->state = IPSEC_SPSTATE_DEAD; - SPTREE_UNLOCK(); + key_unlink(sp); KEY_FREESP(&sp); { @@ -2313,6 +2299,7 @@ key_spdacquire(struct secpolicy *sp) } else { /* increment counter and do nothing. */ newspacq->count++; + SPACQ_UNLOCK(); return 0; } SPACQ_UNLOCK(); @@ -2355,8 +2342,9 @@ key_spdacquire(struct secpolicy *sp) static int key_spdflush(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) { + TAILQ_HEAD(, secpolicy) drainq; struct sadb_msg *newmsg; - struct secpolicy *sp; + struct secpolicy *sp, *nextsp; u_int dir; IPSEC_ASSERT(so != NULL, ("null socket")); @@ -2367,11 +2355,17 @@ key_spdflush(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) if (m->m_len != PFKEY_ALIGN8(sizeof(struct sadb_msg))) return key_senderror(so, m, EINVAL); + TAILQ_INIT(&drainq); + SPTREE_WLOCK(); for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[dir], chain) - sp->state = IPSEC_SPSTATE_DEAD; - SPTREE_UNLOCK(); + TAILQ_CONCAT(&drainq, &V_sptree[dir], chain); + } + SPTREE_WUNLOCK(); + sp = TAILQ_FIRST(&drainq); + while (sp != NULL) { + nextsp = TAILQ_NEXT(sp, chain); + KEY_FREESP(&sp); + sp = nextsp; } if (sizeof(struct sadb_msg) > m->m_len + M_TRAILINGSPACE(m)) { @@ -2404,10 +2398,11 @@ key_spdflush(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) static int key_spddump(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) { + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; - int cnt; - u_int dir; struct mbuf *n; + u_int dir; + int cnt; IPSEC_ASSERT(so != NULL, ("null socket")); IPSEC_ASSERT(m != NULL, ("null mbuf")); @@ -2416,20 +2411,20 @@ key_spddump(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) /* search SPD entry and get buffer size. */ cnt = 0; - SPTREE_LOCK(); + SPTREE_RLOCK(); for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { - LIST_FOREACH(sp, &V_sptree[dir], chain) { + TAILQ_FOREACH(sp, &V_sptree[dir], chain) { cnt++; } } if (cnt == 0) { - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); return key_senderror(so, m, ENOENT); } for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { - LIST_FOREACH(sp, &V_sptree[dir], chain) { + TAILQ_FOREACH(sp, &V_sptree[dir], chain) { --cnt; n = key_setdumpsp(sp, SADB_X_SPDDUMP, cnt, mhp->msg->sadb_msg_pid); @@ -2439,7 +2434,7 @@ key_spddump(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) } } - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); m_freem(m); return 0; } @@ -2451,6 +2446,8 @@ key_setdumpsp(struct secpolicy *sp, u_int8_t type, u_int32_t seq, struct mbuf *result = NULL, *m; struct seclifetime lt; + SPTREE_RLOCK_ASSERT(); + m = key_setsadbmsg(type, 0, SADB_SATYPE_UNSPEC, seq, pid, sp->refcnt); if (!m) goto fail; @@ -2566,8 +2563,6 @@ key_spdexpire(struct secpolicy *sp) int error = -1; struct sadb_lifetime *lt; - /* XXX: Why do we lock ? */ - IPSEC_ASSERT(sp != NULL, ("null secpolicy")); /* set msg header */ @@ -2678,10 +2673,12 @@ key_newsah(struct secasindex *saidx) IPSEC_ASSERT(saidx != NULL, ("null saidx")); - newsah = malloc(sizeof(struct secashead), M_IPSEC_SAH, M_NOWAIT|M_ZERO); + newsah = malloc(sizeof(struct secashead), M_IPSEC_SAH, + M_NOWAIT | M_ZERO); if (newsah != NULL) { int i; - for (i = 0; i < sizeof(newsah->savtree)/sizeof(newsah->savtree[0]); i++) + for (i = 0; i < sizeof(newsah->savtree) / + sizeof(newsah->savtree[0]); i++) LIST_INIT(&newsah->savtree[i]); newsah->saidx = *saidx; @@ -4225,47 +4222,29 @@ key_bbcmp(const void *a1, const void *a2, u_int bits) static void key_flush_spd(time_t now) { - static u_int16_t sptree_scangen = 0; - u_int16_t gen = sptree_scangen++; + SPTREE_RLOCK_TRACKER; struct secpolicy *sp; u_int dir; /* SPD */ for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { restart: - SPTREE_LOCK(); - LIST_FOREACH(sp, &V_sptree[dir], chain) { - if (sp->scangen == gen) /* previously handled */ - continue; - sp->scangen = gen; - if (sp->state == IPSEC_SPSTATE_DEAD && - sp->refcnt == 1) { - /* - * Ensure that we only decrease refcnt once, - * when we're the last consumer. - * Directly call SP_DELREF/key_delsp instead - * of KEY_FREESP to avoid unlocking/relocking - * SPTREE_LOCK before key_delsp: may refcnt - * be increased again during that time ? - * NB: also clean entries created by - * key_spdflush - */ - SP_DELREF(sp); - key_delsp(sp); - SPTREE_UNLOCK(); - goto restart; - } + SPTREE_RLOCK(); + TAILQ_FOREACH(sp, &V_sptree[dir], chain) { if (sp->lifetime == 0 && sp->validtime == 0) continue; - if ((sp->lifetime && now - sp->created > sp->lifetime) - || (sp->validtime && now - sp->lastused > sp->validtime)) { - sp->state = IPSEC_SPSTATE_DEAD; - SPTREE_UNLOCK(); + if ((sp->lifetime && + now - sp->created > sp->lifetime) || + (sp->validtime && + now - sp->lastused > sp->validtime)) { + SPTREE_RUNLOCK(); + key_unlink(sp); key_spdexpire(sp); + KEY_FREESP(&sp); goto restart; } } - SPTREE_UNLOCK(); + SPTREE_RUNLOCK(); } } @@ -7608,7 +7587,7 @@ key_init(void) int i; for (i = 0; i < IPSEC_DIR_MAX; i++) - LIST_INIT(&V_sptree[i]); + TAILQ_INIT(&V_sptree[i]); LIST_INIT(&V_sahtree); @@ -7646,6 +7625,7 @@ key_init(void) void key_destroy(void) { + TAILQ_HEAD(, secpolicy) drainq; struct secpolicy *sp, *nextsp; struct secacq *acq, *nextacq; struct secspacq *spacq, *nextspacq; @@ -7653,18 +7633,18 @@ key_destroy(void) struct secreg *reg; int i; - SPTREE_LOCK(); - for (i = 0; i < IPSEC_DIR_MAX; i++) { - for (sp = LIST_FIRST(&V_sptree[i]); - sp != NULL; sp = nextsp) { - nextsp = LIST_NEXT(sp, chain); - if (__LIST_CHAINED(sp)) { - LIST_REMOVE(sp, chain); - free(sp, M_IPSEC_SP); - } - } + TAILQ_INIT(&drainq); + SPTREE_WLOCK(); + for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { + TAILQ_CONCAT(&drainq, &V_sptree[dir], chain); + } + SPTREE_WUNLOCK(); + sp = TAILQ_FIRST(&drainq); + while (sp != NULL) { + nextsp = TAILQ_NEXT(sp, chain); + KEY_FREESP(&sp); + sp = nextsp; } - SPTREE_UNLOCK(); SAHTREE_LOCK(); for (sah = LIST_FIRST(&V_sahtree); sah != NULL; sah = nextsah) { diff --git a/sys/netipsec/key_debug.c b/sys/netipsec/key_debug.c index 5cbc1db..97ac061 100644 --- a/sys/netipsec/key_debug.c +++ b/sys/netipsec/key_debug.c @@ -463,8 +463,8 @@ kdebug_secpolicy(struct secpolicy *sp) if (sp == NULL) panic("%s: NULL pointer was passed.\n", __func__); - printf("secpolicy{ refcnt=%u state=%u policy=%u\n", - sp->refcnt, sp->state, sp->policy); + printf("secpolicy{ refcnt=%u policy=%u\n", + sp->refcnt, sp->policy); kdebug_secpolicyindex(&sp->spidx);