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 087d63e..35a40af 100644 (file)
--- 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,