diff options
author | ae <ae@FreeBSD.org> | 2015-02-24 10:35:07 +0000 |
---|---|---|
committer | ae <ae@FreeBSD.org> | 2015-02-24 10:35:07 +0000 |
commit | 217cb9ff265db473319230edb49f91a57d80923e (patch) | |
tree | 170e1b146f9a451834c86cbf19dfc1b4bc9bc195 /sys/netipsec/key.c | |
parent | b9f1e32cfa9678cfa78537ef1f085308892fddd4 (diff) | |
download | FreeBSD-src-217cb9ff265db473319230edb49f91a57d80923e.zip FreeBSD-src-217cb9ff265db473319230edb49f91a57d80923e.tar.gz |
Fix possible memory leak and several races in the IPsec policy management
code.
Resurrect the state field in the struct secpolicy, it has
IPSEC_SPSTATE_ALIVE value when security policy linked in the chain,
and IPSEC_SPSTATE_DEAD value in all other cases. This field protects
from trying to unlink one security policy several times from the different
threads.
Take additional reference in the key_flush_spd() to be sure that policy
won't be freed from the different thread while we are sending SPDEXPIRE message.
Add KEY_FREESP() call to the key_unlink() to release additional reference
that we take when use key_getsp*() functions.
Differential Revision: https://reviews.freebsd.org/D1914
Tested by: Emeric POUPON <emeric.poupon at stormshield dot eu>
Reviewed by: hrs
Sponsored by: Yandex LLC
Diffstat (limited to 'sys/netipsec/key.c')
-rw-r--r-- | sys/netipsec/key.c | 16 |
1 files changed, 15 insertions, 1 deletions
diff --git a/sys/netipsec/key.c b/sys/netipsec/key.c index c38dae8..ef06ef4 100644 --- a/sys/netipsec/key.c +++ b/sys/netipsec/key.c @@ -1198,8 +1198,14 @@ key_unlink(struct secpolicy *sp) SPTREE_UNLOCK_ASSERT(); SPTREE_WLOCK(); + if (sp->state == IPSEC_SPSTATE_DEAD) { + SPTREE_WUNLOCK(); + return; + } + sp->state = IPSEC_SPSTATE_DEAD; TAILQ_REMOVE(&V_sptree[sp->spidx.dir], sp, chain); SPTREE_WUNLOCK(); + KEY_FREESP(&sp); } /* @@ -1900,6 +1906,7 @@ key_spdadd(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) SPTREE_WLOCK(); TAILQ_INSERT_TAIL(&V_sptree[newsp->spidx.dir], newsp, chain); + newsp->state = IPSEC_SPSTATE_ALIVE; SPTREE_WUNLOCK(); /* delete the entry in spacqtree */ @@ -2335,6 +2342,12 @@ key_spdflush(struct socket *so, struct mbuf *m, const struct sadb_msghdr *mhp) for (dir = 0; dir < IPSEC_DIR_MAX; dir++) { TAILQ_CONCAT(&drainq, &V_sptree[dir], chain); } + /* + * We need to set state to DEAD for each policy to be sure, + * that another thread won't try to unlink it. + */ + TAILQ_FOREACH(sp, &drainq, chain) + sp->state = IPSEC_SPSTATE_DEAD; SPTREE_WUNLOCK(); sp = TAILQ_FIRST(&drainq); while (sp != NULL) { @@ -4209,9 +4222,10 @@ restart: now - sp->created > sp->lifetime) || (sp->validtime && now - sp->lastused > sp->validtime)) { + SP_ADDREF(sp); SPTREE_RUNLOCK(); - key_unlink(sp); key_spdexpire(sp); + key_unlink(sp); KEY_FREESP(&sp); goto restart; } |