Hi,
I have found a small race in tdb_delete() between TDBF_DELETED,
tdb_unlink() and tdb_cleanspd(). gettdb...() could 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().
I have never triggered it, but it looks like a possible code path.
ok?
bluhm
Index: netinet/ip_spd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_spd.c,v
retrieving revision 1.109
diff -u -p -r1.109 ip_spd.c
--- netinet/ip_spd.c 14 Dec 2021 17:50:37 -0000 1.109
+++ netinet/ip_spd.c 14 Dec 2021 18:42:59 -0000
@@ -453,6 +453,16 @@ ipsp_spd_lookup(struct mbuf *m, int af,
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,
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,
