Hi,
I have started documenting the locking strategy of struct tdb fields.
Note that gettdb_dir() is MP safe now.
In udpencap_ctlinput() we had an unprotected access to tdb_snext.
Grab the tdb_sadb_mtx mutex there. Make the braces consistently
for all these loops.
Move NET_ASSERT_LOCKED() into the functions where the read access
happens.
ok?
bluhm
Index: net/pfkeyv2.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.226
diff -u -p -r1.226 pfkeyv2.c
--- net/pfkeyv2.c 3 Dec 2021 19:04:49 -0000 1.226
+++ net/pfkeyv2.c 7 Dec 2021 22:16:20 -0000
@@ -800,6 +800,8 @@ pfkeyv2_get(struct tdb *tdb, void **head
int rval, i;
void *p;
+ NET_ASSERT_LOCKED();
+
/* Find how much space we need */
i = sizeof(struct sadb_sa) + sizeof(struct sadb_lifetime) +
sizeof(struct sadb_x_counter);
@@ -2347,6 +2349,8 @@ pfkeyv2_expire(struct tdb *tdb, u_int16_
struct sadb_msg *smsg;
int rval = 0;
int i;
+
+ NET_ASSERT_LOCKED();
switch (tdb->tdb_sproto) {
case IPPROTO_AH:
Index: netinet/ip_ipsp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.c,v
retrieving revision 1.262
diff -u -p -r1.262 ip_ipsp.c
--- netinet/ip_ipsp.c 7 Dec 2021 17:28:46 -0000 1.262
+++ netinet/ip_ipsp.c 7 Dec 2021 22:39:59 -0000
@@ -243,8 +243,6 @@ reserve_spi(u_int rdomain, u_int32_t ssp
u_int32_t spi;
int nums;
- NET_ASSERT_LOCKED();
-
/* Don't accept ranges only encompassing reserved SPIs. */
if (sproto != IPPROTO_IPCOMP &&
(tspi < sspi || tspi <= SPI_RESERVED_MAX)) {
@@ -343,6 +341,8 @@ gettdb_dir(u_int rdomain, u_int32_t spi,
u_int32_t hashval;
struct tdb *tdbp;
+ NET_ASSERT_LOCKED();
+
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(spi, dst, proto);
@@ -374,7 +374,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(0, src, proto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if (tdbp->tdb_sproto == proto &&
(spi == 0 || tdbp->tdb_spi == spi) &&
((!reverse && tdbp->tdb_rdomain == rdomain) ||
@@ -384,7 +384,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
!memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
!memcmp(&tdbp->tdb_src, src, src->sa.sa_len))
break;
-
+ }
if (tdbp != NULL) {
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
@@ -395,7 +395,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
su_null.sa.sa_len = sizeof(struct sockaddr);
hashval = tdb_hash(0, &su_null, proto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if (tdbp->tdb_sproto == proto &&
(spi == 0 || tdbp->tdb_spi == spi) &&
((!reverse && tdbp->tdb_rdomain == rdomain) ||
@@ -405,7 +405,7 @@ gettdbbysrcdst_dir(u_int rdomain, u_int3
!memcmp(&tdbp->tdb_dst, dst, dst->sa.sa_len)) &&
tdbp->tdb_src.sa.sa_family == AF_UNSPEC)
break;
-
+ }
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
return tdbp;
@@ -494,7 +494,7 @@ gettdbbysrc(u_int rdomain, union sockadd
mtx_enter(&tdb_sadb_mtx);
hashval = tdb_hash(0, src, sproto);
- for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext)
+ for (tdbp = tdbsrc[hashval]; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if ((tdbp->tdb_sproto == sproto) &&
(tdbp->tdb_rdomain == rdomain) &&
((tdbp->tdb_flags & TDBF_INVALID) == 0) &&
@@ -504,7 +504,7 @@ gettdbbysrc(u_int rdomain, union sockadd
continue;
break;
}
-
+ }
tdb_ref(tdbp);
mtx_leave(&tdb_sadb_mtx);
return tdbp;
@@ -900,8 +900,7 @@ tdb_unlink_locked(struct tdb *tdbp)
if (tdbsrc[hashval] == tdbp) {
tdbsrc[hashval] = tdbp->tdb_snext;
- }
- else {
+ } else {
for (tdbpp = tdbsrc[hashval]; tdbpp != NULL;
tdbpp = tdbpp->tdb_snext) {
if (tdbpp->tdb_snext == tdbp) {
@@ -1031,8 +1030,6 @@ struct tdb *
tdb_alloc(u_int rdomain)
{
struct tdb *tdbp;
-
- NET_ASSERT_LOCKED();
tdbp = pool_get(&tdb_pool, PR_WAITOK | PR_ZERO);
Index: netinet/ip_ipsp.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.228
diff -u -p -r1.228 ip_ipsp.h
--- netinet/ip_ipsp.h 7 Dec 2021 17:28:46 -0000 1.228
+++ netinet/ip_ipsp.h 7 Dec 2021 22:39:50 -0000
@@ -309,6 +309,12 @@ struct ipsec_policy {
#define IPSP_IDENTITY_USERFQDN 3
#define IPSP_IDENTITY_ASN1_DN 4
+/*
+ * Locks used to protect struct members in this file:
+ * I immutable after creation
+ * N net lock
+ * s tdb_sadb_mtx
+ */
struct tdb { /* tunnel descriptor block */
/*
* Each TDB is on three hash tables: one keyed on dst/spi/sproto,
@@ -318,9 +324,9 @@ struct tdb { /* tunnel
descriptor blo
* policy matching. The following three fields maintain the hash
* queues in those three tables.
*/
- struct tdb *tdb_hnext; /* dst/spi/sproto table */
- struct tdb *tdb_dnext; /* dst/sproto table */
- struct tdb *tdb_snext; /* src/sproto table */
+ struct tdb *tdb_hnext; /* [s] dst/spi/sproto table */
+ struct tdb *tdb_dnext; /* [s] dst/sproto table */
+ struct tdb *tdb_snext; /* [s] src/sproto table */
struct tdb *tdb_inext;
struct tdb *tdb_onext;
@@ -390,17 +396,17 @@ struct tdb { /* tunnel
descriptor blo
struct tdb_data tdb_data; /* stats about this TDB */
u_int64_t tdb_cryptoid; /* Crypto session ID */
- u_int32_t tdb_spi; /* SPI */
+ u_int32_t tdb_spi; /* [I] SPI */
u_int16_t tdb_amxkeylen; /* Raw authentication key length */
u_int16_t tdb_emxkeylen; /* Raw encryption key length */
u_int16_t tdb_ivlen; /* IV length */
- u_int8_t tdb_sproto; /* IPsec protocol */
+ u_int8_t tdb_sproto; /* [I] IPsec protocol */
u_int8_t tdb_wnd; /* Replay window */
u_int8_t tdb_satype; /* SA type (RFC2367, PF_KEY) */
u_int8_t tdb_updates; /* pfsync update counter */
- union sockaddr_union tdb_dst; /* Destination address */
- union sockaddr_union tdb_src; /* Source address */
+ union sockaddr_union tdb_dst; /* [IN] Destination address */
+ union sockaddr_union tdb_src; /* [IN] Source address */
u_int8_t *tdb_amxkey; /* Raw authentication key */
u_int8_t *tdb_emxkey; /* Raw encryption key */
@@ -424,8 +430,8 @@ struct tdb { /* tunnel
descriptor blo
u_int16_t tdb_tag; /* Packet filter tag */
u_int32_t tdb_tap; /* Alternate enc(4) interface */
- u_int tdb_rdomain; /* Routing domain */
- u_int tdb_rdomain_post; /* Change domain */
+ u_int tdb_rdomain; /* [I] Routing domain */
+ u_int tdb_rdomain_post; /* [I] Change domain */
struct sockaddr_encap tdb_filter; /* What traffic is acceptable */
struct sockaddr_encap tdb_filtermask; /* And the mask */
Index: netinet/ipsec_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.196
diff -u -p -r1.196 ipsec_input.c
--- netinet/ipsec_input.c 2 Dec 2021 13:46:42 -0000 1.196
+++ netinet/ipsec_input.c 7 Dec 2021 21:09:49 -0000
@@ -934,15 +934,16 @@ udpencap_ctlinput(int cmd, struct sockad
first = gettdbbysrcdst_rev(rdomain, 0, su_src, su_dst, IPPROTO_ESP);
+ mtx_enter(&tdb_sadb_mtx);
for (tdbp = first; tdbp != NULL; tdbp = tdbp->tdb_snext) {
if (tdbp->tdb_sproto == IPPROTO_ESP &&
((tdbp->tdb_flags & (TDBF_INVALID|TDBF_UDPENCAP)) ==
TDBF_UDPENCAP) &&
!memcmp(&tdbp->tdb_dst, &dst, su_dst->sa.sa_len) &&
- !memcmp(&tdbp->tdb_src, &src, su_src->sa.sa_len)) {
+ !memcmp(&tdbp->tdb_src, &src, su_src->sa.sa_len))
ipsec_set_mtu(tdbp, mtu);
- }
}
+ mtx_leave(&tdb_sadb_mtx);
tdb_unref(first);
}
