From acf1bec55e9ad79e5385eab3e58702adcd02a571 Mon Sep 17 00:00:00 2001 From: tobhe Date: Fri, 26 Nov 2021 16:16:35 +0000 Subject: [PATCH] Replace TDBF_DELETED flag with check if tdb was already unlinked. Protect tdb_unlink() and puttdb() for SADB_UPDATE with tdb_sadb_mutex. Tested by Hrvoje Popovski ok bluhm@ mvs@ --- sys/net/pfkeyv2.c | 13 ++++++------- sys/netinet/ip_ipsp.c | 31 +++++++++++++++++++++---------- sys/netinet/ip_ipsp.h | 12 +++++++----- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/sys/net/pfkeyv2.c b/sys/net/pfkeyv2.c index ac1d4376135..b47e19e2a79 100644 --- a/sys/net/pfkeyv2.c +++ b/sys/net/pfkeyv2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfkeyv2.c,v 1.222 2021/11/25 13:46:02 bluhm Exp $ */ +/* $OpenBSD: pfkeyv2.c,v 1.223 2021/11/26 16:16:35 tobhe Exp $ */ /* * @(#)COPYRIGHT 1.1 (NRL) 17 January 1995 @@ -1046,11 +1046,8 @@ pfkeyv2_sa_flush(struct tdb *tdb, void *satype_vp, int last) /* keep in sync with tdb_delete() */ NET_ASSERT_LOCKED(); - if (tdb->tdb_flags & TDBF_DELETED) + if (tdb_unlink_locked(tdb) == 0) return (0); - tdb->tdb_flags |= TDBF_DELETED; - - tdb_unlink_locked(tdb); tdb_unbundle(tdb); tdb_deltimeouts(tdb); tdb_unref(tdb); @@ -1438,12 +1435,14 @@ pfkeyv2_send(struct socket *so, void *message, int len) #endif if (headers[SADB_EXT_ADDRESS_SRC] || headers[SADB_EXT_ADDRESS_PROXY]) { - tdb_unlink(sa2); + mtx_enter(&tdb_sadb_mtx); + tdb_unlink_locked(sa2); import_address((struct sockaddr *)&sa2->tdb_src, headers[SADB_EXT_ADDRESS_SRC]); import_address((struct sockaddr *)&sa2->tdb_dst, headers[SADB_EXT_ADDRESS_PROXY]); - puttdb(sa2); + puttdb_locked(sa2); + mtx_leave(&tdb_sadb_mtx); } } NET_UNLOCK(); diff --git a/sys/netinet/ip_ipsp.c b/sys/netinet/ip_ipsp.c index 1304a7ce255..bd84e9f742b 100644 --- a/sys/netinet/ip_ipsp.c +++ b/sys/netinet/ip_ipsp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.c,v 1.254 2021/11/25 13:46:02 bluhm Exp $ */ +/* $OpenBSD: ip_ipsp.c,v 1.255 2021/11/26 16:16:35 tobhe Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -793,10 +793,17 @@ tdb_rehash(void) */ void puttdb(struct tdb *tdbp) +{ + mtx_enter(&tdb_sadb_mtx); + puttdb_locked(tdbp); + mtx_leave(&tdb_sadb_mtx); +} + +void +puttdb_locked(struct tdb *tdbp) { u_int32_t hashval; - mtx_enter(&tdb_sadb_mtx); hashval = tdb_hash(tdbp->tdb_spi, &tdbp->tdb_dst, tdbp->tdb_sproto); /* @@ -832,18 +839,20 @@ puttdb(struct tdb *tdbp) #endif /* IPSEC */ ipsec_last_added = getuptime(); - mtx_leave(&tdb_sadb_mtx); } -void +int tdb_unlink(struct tdb *tdbp) { + int r; + mtx_enter(&tdb_sadb_mtx); - tdb_unlink_locked(tdbp); + r = tdb_unlink_locked(tdbp); mtx_leave(&tdb_sadb_mtx); + return (r); } -void +int tdb_unlink_locked(struct tdb *tdbp) { struct tdb *tdbpp; @@ -851,6 +860,9 @@ tdb_unlink_locked(struct tdb *tdbp) MUTEX_ASSERT_LOCKED(&tdb_sadb_mtx); + if (tdbp->tdb_dnext == NULL && tdbp->tdb_snext == NULL) + return (0); + hashval = tdb_hash(tdbp->tdb_spi, &tdbp->tdb_dst, tdbp->tdb_sproto); if (tdbh[hashval] == tdbp) { @@ -907,6 +919,8 @@ tdb_unlink_locked(struct tdb *tdbp) ipsecstat_inc(ipsec_prevtunnels); } #endif /* IPSEC */ + + return (1); } void @@ -968,11 +982,8 @@ tdb_delete(struct tdb *tdbp) /* keep in sync with pfkeyv2_sa_flush() */ NET_ASSERT_LOCKED(); - if (tdbp->tdb_flags & TDBF_DELETED) + if (tdb_unlink(tdbp) == 0) return; - tdbp->tdb_flags |= TDBF_DELETED; - - tdb_unlink(tdbp); /* release tdb_onext/tdb_inext references */ tdb_unbundle(tdbp); /* delete timeouts and release references */ diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h index 46fd13b610b..b0559ff2acf 100644 --- a/sys/netinet/ip_ipsp.h +++ b/sys/netinet/ip_ipsp.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ip_ipsp.h,v 1.222 2021/11/25 13:46:02 bluhm Exp $ */ +/* $OpenBSD: ip_ipsp.h,v 1.223 2021/11/26 16:16:35 tobhe Exp $ */ /* * The authors of this code are John Ioannidis (ji@tla.org), * Angelos D. Keromytis (kermit@csd.uch.gr), @@ -337,7 +337,6 @@ struct tdb { /* tunnel descriptor block */ #define TDBF_ALLOCATIONS 0x00008 /* Check the flows counters */ #define TDBF_INVALID 0x00010 /* This SPI is not valid yet/anymore */ #define TDBF_FIRSTUSE 0x00020 /* Expire after first use */ -#define TDBF_DELETED 0x00040 /* This TDB has already been deleted */ #define TDBF_SOFT_TIMER 0x00080 /* Soft expiration */ #define TDBF_SOFT_BYTES 0x00100 /* Soft expiration */ #define TDBF_SOFT_ALLOCATIONS 0x00200 /* Soft expiration */ @@ -352,7 +351,7 @@ struct tdb { /* tunnel descriptor block */ #define TDBF_BITS ("\20" \ "\1UNIQUE\2TIMER\3BYTES\4ALLOCATIONS" \ - "\5INVALID\6FIRSTUSE\7DELETED\10SOFT_TIMER" \ + "\5INVALID\6FIRSTUSE\10SOFT_TIMER" \ "\11SOFT_BYTES\12SOFT_ALLOCATIONS\13SOFT_FIRSTUSE\14PFS" \ "\15TUNNELING" \ "\21USEDTUNNEL\22UDPENCAP\23PFSYNC\24PFSYNC_RPL" \ @@ -537,6 +536,8 @@ extern char ipsec_def_comp[]; extern TAILQ_HEAD(ipsec_policy_head, ipsec_policy) ipsec_policy_head; +extern struct mutex tdb_sadb_mtx; + struct cryptop; /* Misc. */ @@ -565,14 +566,15 @@ struct tdb *gettdbbysrcdst_dir(u_int, u_int32_t, union sockaddr_union *, #define gettdbbysrcdst(a,b,c,d,e) gettdbbysrcdst_dir((a),(b),(c),(d),(e),0) #define gettdbbysrcdst_rev(a,b,c,d,e) gettdbbysrcdst_dir((a),(b),(c),(d),(e),1) void puttdb(struct tdb *); +void puttdb_locked(struct tdb *); void tdb_delete(struct tdb *); struct tdb *tdb_alloc(u_int); struct tdb *tdb_ref(struct tdb *); void tdb_unref(struct tdb *); void tdb_free(struct tdb *); int tdb_init(struct tdb *, u_int16_t, struct ipsecinit *); -void tdb_unlink(struct tdb *); -void tdb_unlink_locked(struct tdb *); +int tdb_unlink(struct tdb *); +int tdb_unlink_locked(struct tdb *); void tdb_unbundle(struct tdb *); void tdb_deltimeouts(struct tdb *); int tdb_walk(u_int, int (*)(struct tdb *, void *, int), void *); -- 2.20.1