From: tobhe <tobhe@openbsd.org>
Date: Fri, 26 Nov 2021 16:16:35 +0000 (+0000)
Subject: Replace TDBF_DELETED flag with check if tdb was already unlinked.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=acf1bec55e9ad79e5385eab3e58702adcd02a571;p=openbsd

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@
---

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 *);