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
This commit is contained in:
parent
bcb1c16516
commit
47568136c5
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=279234
@ -89,6 +89,9 @@ struct secpolicy {
|
||||
/* if policy == IPSEC else this value == NULL.*/
|
||||
u_int refcnt; /* reference count */
|
||||
u_int policy; /* policy_type per pfkeyv2.h */
|
||||
u_int state;
|
||||
#define IPSEC_SPSTATE_DEAD 0
|
||||
#define IPSEC_SPSTATE_ALIVE 1
|
||||
u_int32_t id; /* It's unique number on the system. */
|
||||
/*
|
||||
* lifetime handler.
|
||||
|
@ -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 @@ key_flush_spd(time_t now)
|
||||
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;
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user