From: bluhm <bluhm@openbsd.org>
Date: Thu, 16 Dec 2021 15:38:03 +0000 (+0000)
Subject: Fix a tiny race in tdb_delete() between TDBF_DELETED, tdb_unlink()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f40da5574981c10871821bfe1271389abcca31ae;p=openbsd

Fix a tiny race in tdb_delete() between TDBF_DELETED, tdb_unlink()
and tdb_cleanspd().  gettdb...() can return a TDB before tdb_unlink().
Then ipsp_spd_lookup() could add it to tdb_policy_head after
tdb_cleanspd().  There it would stay until it hits the kassert in
tdb_free().
OK tobhe@
---

diff --git a/sys/netinet/ip_spd.c b/sys/netinet/ip_spd.c
index 087d63ef6c2..35a40afa2ef 100644
--- a/sys/netinet/ip_spd.c
+++ b/sys/netinet/ip_spd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_spd.c,v 1.109 2021/12/14 17:50:37 bluhm Exp $ */
+/* $OpenBSD: ip_spd.c,v 1.110 2021/12/16 15:38:03 bluhm Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -453,6 +453,16 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
 			    ipo->ipo_sproto, ids ? ids: ipo->ipo_ids,
 			    &ipo->ipo_addr, &ipo->ipo_mask);
 			mtx_enter(&ipo_tdb_mtx);
+			if ((tdbp_new != NULL) &&
+			    (tdbp_new->tdb_flags & TDBF_DELETED)) {
+				/*
+				 * After tdb_delete() has released ipo_tdb_mtx
+				 * in tdb_unlink(), never add a new one.
+				 * tdb_cleanspd() has to catch all of them.
+				 */
+				tdb_unref(tdbp_new);
+				tdbp_new = NULL;
+			}
 			if (ipo->ipo_tdb != NULL) {
 				/* Remove cached TDB from parallel thread. */
 				TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head,
@@ -590,6 +600,16 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
 			    ipo->ipo_sproto, ipo->ipo_ids,
 			    &ipo->ipo_addr, &ipo->ipo_mask);
 			mtx_enter(&ipo_tdb_mtx);
+			if ((tdbp_new != NULL) &&
+			    (tdbp_new->tdb_flags & TDBF_DELETED)) {
+				/*
+				 * After tdb_delete() has released ipo_tdb_mtx
+				 * in tdb_unlink(), never add a new one.
+				 * tdb_cleanspd() has to catch all of them.
+				 */
+				tdb_unref(tdbp_new);
+				tdbp_new = NULL;
+			}
 			if (ipo->ipo_tdb != NULL) {
 				/* Remove cached TDB from parallel thread. */
 				TAILQ_REMOVE(&ipo->ipo_tdb->tdb_policy_head,