Hi,
I would like to protect the write access to the TDB flags field
with a mutex. Clearing the timeout flags just before pool put in
tdb_free() does not make sense. Move this to tdb_delete().
While there make the parentheses in the flag check consistent.
ok?
bluhm
Index: net/pfkeyv2_convert.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2_convert.c,v
retrieving revision 1.76
diff -u -p -r1.76 pfkeyv2_convert.c
--- net/pfkeyv2_convert.c 25 Nov 2021 13:46:02 -0000 1.76
+++ net/pfkeyv2_convert.c 9 Dec 2021 15:15:34 -0000
@@ -122,6 +122,7 @@ import_sa(struct tdb *tdb, struct sadb_s
if (!sadb_sa)
return;
+ mtx_enter(&tdb->tdb_mtx);
if (ii) {
ii->ii_encalg = sadb_sa->sadb_sa_encrypt;
ii->ii_authalg = sadb_sa->sadb_sa_auth;
@@ -145,6 +146,7 @@ import_sa(struct tdb *tdb, struct sadb_s
if (sadb_sa->sadb_sa_state != SADB_SASTATE_MATURE)
tdb->tdb_flags |= TDBF_INVALID;
+ mtx_leave(&tdb->tdb_mtx);
}
/*
@@ -282,6 +284,7 @@ import_lifetime(struct tdb *tdb, struct
if (!sadb_lifetime)
return;
+ mtx_enter(&tdb->tdb_mtx);
switch (type) {
case PFKEYV2_LIFETIME_HARD:
if ((tdb->tdb_exp_allocations =
@@ -348,6 +351,7 @@ import_lifetime(struct tdb *tdb, struct
tdb->tdb_established = sadb_lifetime->sadb_lifetime_addtime;
tdb->tdb_first_use = sadb_lifetime->sadb_lifetime_usetime;
}
+ mtx_leave(&tdb->tdb_mtx);
}
/*
Index: netinet/ip_ah.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.168
diff -u -p -r1.168 ip_ah.c
--- netinet/ip_ah.c 2 Dec 2021 12:39:15 -0000 1.168
+++ netinet/ip_ah.c 9 Dec 2021 17:46:28 -0000
@@ -612,8 +612,8 @@ ah_input(struct mbuf **mp, struct tdb *t
ahstat_add(ahs_ibytes, ibytes);
/* Hard expiration. */
- if (tdb->tdb_flags & TDBF_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+ if ((tdb->tdb_flags & TDBF_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
ipsecstat_inc(ipsec_exctdb);
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
@@ -621,11 +621,15 @@ ah_input(struct mbuf **mp, struct tdb *t
}
/* Notify on expiration. */
- if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+ mtx_enter(&tdb->tdb_mtx);
+ if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking. */
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/* Get crypto descriptors. */
crp = crypto_getreq(1);
@@ -952,8 +956,8 @@ ah_output(struct mbuf *m, struct tdb *td
ahstat_add(ahs_obytes, m->m_pkthdr.len - skip);
/* Hard expiration. */
- if (tdb->tdb_flags & TDBF_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+ if ((tdb->tdb_flags & TDBF_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
ipsecstat_inc(ipsec_exctdb);
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
@@ -962,11 +966,15 @@ ah_output(struct mbuf *m, struct tdb *td
}
/* Notify on expiration. */
- if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+ mtx_enter(&tdb->tdb_mtx);
+ if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/*
* Loop through mbuf chain; if we find a readonly mbuf,
Index: netinet/ip_esp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_esp.c,v
retrieving revision 1.188
diff -u -p -r1.188 ip_esp.c
--- netinet/ip_esp.c 21 Nov 2021 16:17:48 -0000 1.188
+++ netinet/ip_esp.c 9 Dec 2021 17:47:28 -0000
@@ -433,11 +433,15 @@ esp_input(struct mbuf **mp, struct tdb *
}
/* Notify on soft expiration */
+ mtx_enter(&tdb->tdb_mtx);
if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
(tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking
*/
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/* Get crypto descriptors */
crp = crypto_getreq(esph && espx ? 2 : 1);
@@ -781,8 +785,8 @@ esp_output(struct mbuf *m, struct tdb *t
espstat_add(esps_obytes, m->m_pkthdr.len - skip);
/* Hard byte expiration. */
- if (tdb->tdb_flags & TDBF_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes) {
+ if ((tdb->tdb_flags & TDBF_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_exp_bytes)) {
ipsecstat_inc(ipsec_exctdb);
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_HARD);
tdb_delete(tdb);
@@ -791,11 +795,15 @@ esp_output(struct mbuf *m, struct tdb *t
}
/* Soft byte expiration. */
- if (tdb->tdb_flags & TDBF_SOFT_BYTES &&
- tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes) {
+ mtx_enter(&tdb->tdb_mtx);
+ if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
+ (tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking. */
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/*
* Loop through mbuf chain; if we find a readonly mbuf,
Index: netinet/ip_ipcomp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipcomp.c,v
retrieving revision 1.88
diff -u -p -r1.88 ip_ipcomp.c
--- netinet/ip_ipcomp.c 21 Nov 2021 16:17:48 -0000 1.88
+++ netinet/ip_ipcomp.c 9 Dec 2021 17:48:06 -0000
@@ -205,11 +205,15 @@ ipcomp_input(struct mbuf **mp, struct td
goto drop;
}
/* Notify on soft expiration */
+ mtx_enter(&tdb->tdb_mtx);
if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
(tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/* In case it's not done already, adjust the size of the mbuf chain */
m->m_pkthdr.len = clen + hlen + skip;
@@ -393,12 +397,18 @@ ipcomp_output(struct mbuf *m, struct tdb
error = EINVAL;
goto drop;
}
+
/* Soft byte expiration */
+ mtx_enter(&tdb->tdb_mtx);
if ((tdb->tdb_flags & TDBF_SOFT_BYTES) &&
(tdb->tdb_cur_bytes >= tdb->tdb_soft_bytes)) {
+ tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
+ mtx_leave(&tdb->tdb_mtx);
+ /* may sleep in solock() for the pfkey socket */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_BYTES; /* Turn off checking */
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
+
/*
* Loop through mbuf chain; if we find a readonly mbuf,
* copy the packet.
Index: netinet/ip_ipsp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.263
diff -u -p -r1.263 ip_ipsp.c
--- netinet/ip_ipsp.c 8 Dec 2021 14:24:18 -0000 1.263
+++ netinet/ip_ipsp.c 9 Dec 2021 17:50:12 -0000
@@ -311,11 +311,13 @@ reserve_spi(u_int rdomain, u_int32_t ssp
#ifdef IPSEC
/* Setup a "silent" expiration (since TDBF_INVALID's set). */
if (ipsec_keep_invalid > 0) {
+ mtx_enter(&tdbp->tdb_mtx);
tdbp->tdb_flags |= TDBF_TIMER;
tdbp->tdb_exp_timeout = ipsec_keep_invalid;
if (timeout_add_sec(&tdbp->tdb_timer_tmo,
ipsec_keep_invalid))
tdb_ref(tdbp);
+ mtx_leave(&tdbp->tdb_mtx);
}
#endif
@@ -701,11 +703,14 @@ tdb_soft_timeout(void *v)
struct tdb *tdb = v;
NET_LOCK();
+ mtx_enter(&tdb->tdb_mtx);
if (tdb->tdb_flags & TDBF_SOFT_TIMER) {
+ tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
+ mtx_leave(&tdb->tdb_mtx);
/* Soft expirations. */
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_TIMER;
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/* decrement refcount of the timeout argument */
tdb_unref(tdb);
NET_UNLOCK();
@@ -717,12 +722,15 @@ tdb_soft_firstuse(void *v)
struct tdb *tdb = v;
NET_LOCK();
+ mtx_enter(&tdb->tdb_mtx);
if (tdb->tdb_flags & TDBF_SOFT_FIRSTUSE) {
+ tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
+ mtx_leave(&tdb->tdb_mtx);
/* If the TDB hasn't been used, don't renew it. */
if (tdb->tdb_first_use != 0)
pfkeyv2_expire(tdb, SADB_EXT_LIFETIME_SOFT);
- tdb->tdb_flags &= ~TDBF_SOFT_FIRSTUSE;
- }
+ } else
+ mtx_leave(&tdb->tdb_mtx);
/* decrement refcount of the timeout argument */
tdb_unref(tdb);
NET_UNLOCK();
@@ -958,6 +966,9 @@ tdb_unbundle(struct tdb *tdbp)
void
tdb_deltimeouts(struct tdb *tdbp)
{
+ mtx_enter(&tdbp->tdb_mtx);
+ tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
+ TDBF_SOFT_TIMER);
if (timeout_del(&tdbp->tdb_timer_tmo))
tdb_unref(tdbp);
if (timeout_del(&tdbp->tdb_first_tmo))
@@ -966,6 +977,7 @@ tdb_deltimeouts(struct tdb *tdbp)
tdb_unref(tdbp);
if (timeout_del(&tdbp->tdb_sfirst_tmo))
tdb_unref(tdbp);
+ mtx_leave(&tdbp->tdb_mtx);
}
struct tdb *
@@ -1005,9 +1017,13 @@ tdb_dodelete(struct tdb *tdbp, int locke
{
NET_ASSERT_LOCKED();
- if (tdbp->tdb_flags & TDBF_DELETED)
+ mtx_enter(&tdbp->tdb_mtx);
+ if (tdbp->tdb_flags & TDBF_DELETED) {
+ mtx_leave(&tdbp->tdb_mtx);
return;
+ }
tdbp->tdb_flags |= TDBF_DELETED;
+ mtx_leave(&tdbp->tdb_mtx);
if (locked)
tdb_unlink_locked(tdbp);
else
@@ -1034,6 +1050,7 @@ tdb_alloc(u_int rdomain)
tdbp = pool_get(&tdb_pool, PR_WAITOK | PR_ZERO);
refcnt_init(&tdbp->tdb_refcnt);
+ mtx_init(&tdbp->tdb_mtx, IPL_SOFTNET);
TAILQ_INIT(&tdbp->tdb_policy_head);
/* Record establishment time. */
@@ -1085,8 +1102,6 @@ tdb_free(struct tdb *tdbp)
KASSERT(tdbp->tdb_inext == NULL);
/* Remove expiration timeouts. */
- tdbp->tdb_flags &= ~(TDBF_FIRSTUSE | TDBF_SOFT_FIRSTUSE | TDBF_TIMER |
- TDBF_SOFT_TIMER);
KASSERT(timeout_pending(&tdbp->tdb_timer_tmo) == 0);
KASSERT(timeout_pending(&tdbp->tdb_first_tmo) == 0);
KASSERT(timeout_pending(&tdbp->tdb_stimer_tmo) == 0);
Index: netinet/ip_ipsp.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.229
diff -u -p -r1.229 ip_ipsp.h
--- netinet/ip_ipsp.h 8 Dec 2021 14:24:18 -0000 1.229
+++ netinet/ip_ipsp.h 9 Dec 2021 13:55:59 -0000
@@ -314,6 +314,7 @@ struct ipsec_policy {
* I immutable after creation
* N net lock
* s tdb_sadb_mtx
+ * m tdb_mtx
*/
struct tdb { /* tunnel descriptor block */
/*
@@ -331,6 +332,7 @@ struct tdb { /* tunnel
descriptor blo
struct tdb *tdb_onext;
struct refcnt tdb_refcnt;
+ struct mutex tdb_mtx;
const struct xformsw *tdb_xform; /* Transform to use */
const struct enc_xform *tdb_encalgxform; /* Enc algorithm */
@@ -364,7 +366,7 @@ struct tdb { /* tunnel
descriptor blo
"\21USEDTUNNEL\22UDPENCAP\23PFSYNC\24PFSYNC_RPL" \
"\25ESN")
- u_int32_t tdb_flags; /* Flags related to this TDB */
+ u_int32_t tdb_flags; /* [m] Flags related to this TDB */
struct timeout tdb_timer_tmo;
struct timeout tdb_first_tmo;
Index: netinet/ipsec_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_output.c,v
retrieving revision 1.93
diff -u -p -r1.93 ipsec_output.c
--- netinet/ipsec_output.c 2 Dec 2021 12:39:15 -0000 1.93
+++ netinet/ipsec_output.c 9 Dec 2021 15:19:22 -0000
@@ -268,7 +268,9 @@ ipsp_process_packet(struct mbuf *m, stru
}
/* Remember that we appended a tunnel header. */
+ mtx_enter(&tdb->tdb_mtx);
tdb->tdb_flags |= TDBF_USEDTUNNEL;
+ mtx_leave(&tdb->tdb_mtx);
}
}
