Add `ipsec_flows_mtx' mutex(9) to protect `ipsp_ids_*' list and

trees.  ipsp_ids_lookup() returns `ids' with bumped reference
counter.  original diff from mvs

ok mvs

10 files changed, 83 insertions, 52 deletions

diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 68cb283945b..2228c241b44 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_bridge.c,v 1.362 2021/12/23 12:21:48 bluhm Exp $	*/
+/*	$OpenBSD: if_bridge.c,v 1.363 2022/01/04 06:32:39 yasuoka Exp $	*/
 
 /*
  * Copyright (c) 1999, 2000 Jason L. Wright (jason@thought.net)
@@ -1595,7 +1595,7 @@ bridge_ipsec(struct ifnet *ifp, struct ether_header *eh, int hassnap,
 		}
 	} else { /* Outgoing from the bridge. */
 		error = ipsp_spd_lookup(m, af, hlen, IPSP_DIRECTION_OUT,
-		    NULL, NULL, &tdb, 0);
+		    NULL, NULL, &tdb, NULL);
 		if (error == 0 && tdb != NULL) {
 			/*
 			 * We don't need to do loop detection, the
diff --git a/sys/net/if_veb.c b/sys/net/if_veb.c
index 97e039b15dd..2976cc200f1 100644
--- a/sys/net/if_veb.c
+++ b/sys/net/if_veb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_veb.c,v 1.24 2021/12/28 23:13:20 dlg Exp $ */
+/*	$OpenBSD: if_veb.c,v 1.25 2022/01/04 06:32:39 yasuoka Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -749,7 +749,7 @@ veb_ipsec_proto_out(struct mbuf *m, sa_family_t af, int iphlen)
 #endif
 
 	tdb = ipsp_spd_lookup(m, af, iphlen, &error, IPSP_DIRECTION_OUT,
-	    NULL, NULL, 0);
+	    NULL, NULL, NULL);
 	if (tdb == NULL)
 		return (m);
 
diff --git a/sys/netinet/ip_ipsp.c b/sys/netinet/ip_ipsp.c
index 66baca79efc..7b1b045e2de 100644
--- a/sys/netinet/ip_ipsp.c
+++ b/sys/netinet/ip_ipsp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ipsp.c,v 1.267 2021/12/20 15:59:09 mvs Exp $	*/
+/*	$OpenBSD: ip_ipsp.c,v 1.268 2022/01/04 06:32:39 yasuoka Exp $	*/
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr),
@@ -47,6 +47,8 @@
 #include <sys/kernel.h>
 #include <sys/timeout.h>
 #include <sys/pool.h>
+#include <sys/atomic.h>
+#include <sys/mutex.h>
 
 #include <net/if.h>
 #include <net/route.h>
@@ -84,6 +86,13 @@ void tdb_hashstats(void);
 	do { } while (0)
 #endif
 
+/*
+ * Locks used to protect global data and struct members:
+ *	F	ipsec_flows_mtx
+ */
+
+struct mutex ipsec_flows_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
+
 int		tdb_rehash(void);
 void		tdb_timeout(void *);
 void		tdb_firstuse(void *);
@@ -98,16 +107,16 @@ int ipsec_ids_idle = 100;		/* keep free ids for 100s */
 struct pool tdb_pool;
 
 /* Protected by the NET_LOCK(). */
-u_int32_t ipsec_ids_next_flow = 1;	/* may not be zero */
-struct ipsec_ids_tree ipsec_ids_tree;
-struct ipsec_ids_flows ipsec_ids_flows;
+u_int32_t ipsec_ids_next_flow = 1;		/* [F] may not be zero */
+struct ipsec_ids_tree ipsec_ids_tree;		/* [F] */
+struct ipsec_ids_flows ipsec_ids_flows;		/* [F] */
 struct ipsec_policy_head ipsec_policy_head =
     TAILQ_HEAD_INITIALIZER(ipsec_policy_head);
 
 void ipsp_ids_gc(void *);
 
 LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
-    LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);
+    LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);	/* [F] */
 struct timeout ipsp_ids_gc_timeout =
     TIMEOUT_INITIALIZER_FLAGS(ipsp_ids_gc, NULL, TIMEOUT_PROC);
 
@@ -1191,21 +1200,25 @@ ipsp_ids_insert(struct ipsec_ids *ids)
 	struct ipsec_ids *found;
 	u_int32_t start_flow;
 
-	NET_ASSERT_LOCKED();
+	mtx_enter(&ipsec_flows_mtx);
 
 	found = RBT_INSERT(ipsec_ids_tree, &ipsec_ids_tree, ids);
 	if (found) {
 		/* if refcount was zero, then timeout is running */
-		if (found->id_refcount++ == 0) {
+		if (atomic_inc_int_nv(&found->id_refcount) == 1) {
 			LIST_REMOVE(found, id_gc_list);
 
 			if (LIST_EMPTY(&ipsp_ids_gc_list))
 				timeout_del(&ipsp_ids_gc_timeout);
 		}
+		mtx_leave (&ipsec_flows_mtx);
 		DPRINTF("ids %p count %d", found, found->id_refcount);
 		return found;
 	}
+
+	ids->id_refcount = 1;
 	ids->id_flow = start_flow = ipsec_ids_next_flow;
+
 	if (++ipsec_ids_next_flow == 0)
 		ipsec_ids_next_flow = 1;
 	while (RBT_INSERT(ipsec_ids_flows, &ipsec_ids_flows, ids) != NULL) {
@@ -1214,12 +1227,13 @@ ipsp_ids_insert(struct ipsec_ids *ids)
 			ipsec_ids_next_flow = 1;
 		if (ipsec_ids_next_flow == start_flow) {
 			RBT_REMOVE(ipsec_ids_tree, &ipsec_ids_tree, ids);
+			mtx_leave(&ipsec_flows_mtx);
 			DPRINTF("ipsec_ids_next_flow exhausted %u",
-			    ipsec_ids_next_flow);
+			    start_flow);
 			return NULL;
 		}
 	}
-	ids->id_refcount = 1;
+	mtx_leave(&ipsec_flows_mtx);
 	DPRINTF("new ids %p flow %u", ids, ids->id_flow);
 	return ids;
 }
@@ -1228,11 +1242,16 @@ struct ipsec_ids *
 ipsp_ids_lookup(u_int32_t ipsecflowinfo)
 {
 	struct ipsec_ids	key;
-
-	NET_ASSERT_LOCKED();
+	struct ipsec_ids	*ids;
 
 	key.id_flow = ipsecflowinfo;
-	return RBT_FIND(ipsec_ids_flows, &ipsec_ids_flows, &key);
+
+	mtx_enter(&ipsec_flows_mtx);
+	ids = RBT_FIND(ipsec_ids_flows, &ipsec_ids_flows, &key);
+	atomic_inc_int(&ids->id_refcount);
+	mtx_leave(&ipsec_flows_mtx);
+
+	return ids;
 }
 
 /* free ids only from delayed timeout */
@@ -1241,7 +1260,7 @@ ipsp_ids_gc(void *arg)
 {
 	struct ipsec_ids *ids, *tids;
 
-	NET_LOCK();
+	mtx_enter(&ipsec_flows_mtx);
 
 	LIST_FOREACH_SAFE(ids, &ipsp_ids_gc_list, id_gc_list, tids) {
 		KASSERT(ids->id_refcount == 0);
@@ -1261,14 +1280,15 @@ ipsp_ids_gc(void *arg)
 	if (!LIST_EMPTY(&ipsp_ids_gc_list))
 		timeout_add_sec(&ipsp_ids_gc_timeout, 1);
 
-	NET_UNLOCK();
+	mtx_leave(&ipsec_flows_mtx);
 }
 
 /* decrements refcount, actual free happens in gc */
 void
 ipsp_ids_free(struct ipsec_ids *ids)
 {
-	NET_ASSERT_LOCKED();
+	if (ids == NULL)
+		return;
 
 	/*
 	 * If the refcount becomes zero, then a timeout is started. This
@@ -1277,9 +1297,11 @@ ipsp_ids_free(struct ipsec_ids *ids)
 	DPRINTF("ids %p count %d", ids, ids->id_refcount);
 	KASSERT(ids->id_refcount > 0);
 
-	if (--ids->id_refcount > 0)
+	if (atomic_dec_int_nv(&ids->id_refcount) > 0)
 		return;
 
+	mtx_enter(&ipsec_flows_mtx);
+
 	/*
 	 * Add second for the case ipsp_ids_gc() is already running and
 	 * awaits netlock to be released.
@@ -1289,6 +1311,8 @@ ipsp_ids_free(struct ipsec_ids *ids)
 	if (LIST_EMPTY(&ipsp_ids_gc_list))
 		timeout_add_sec(&ipsp_ids_gc_timeout, 1);
 	LIST_INSERT_HEAD(&ipsp_ids_gc_list, ids, id_gc_list);
+
+	mtx_leave(&ipsec_flows_mtx);
 }
 
 static int
diff --git a/sys/netinet/ip_ipsp.h b/sys/netinet/ip_ipsp.h
index 1a165b668d0..d4cdbd77f68 100644
--- a/sys/netinet/ip_ipsp.h
+++ b/sys/netinet/ip_ipsp.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ipsp.h,v 1.233 2021/12/20 15:59:10 mvs Exp $	*/
+/*	$OpenBSD: ip_ipsp.h,v 1.234 2022/01/04 06:32:39 yasuoka Exp $	*/
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr),
@@ -40,6 +40,14 @@
 #ifndef _NETINET_IPSP_H_
 #define _NETINET_IPSP_H_
 
+/*
+ * Locks used to protect struct members in this file:
+ *	I	Immutable after creation
+ *	F	ipsec_flows_mtx
+ *	a	atomic
+ *	p	ipo_tdb_mtx		link policy to TDB global mutex
+ */
+
 /* IPSP global definitions. */
 
 #include <sys/types.h>
@@ -223,14 +231,14 @@ struct ipsec_id {
 };
 
 struct ipsec_ids {
-	LIST_ENTRY(ipsec_ids)	id_gc_list;
-	RBT_ENTRY(ipsec_ids)	id_node_id;
-	RBT_ENTRY(ipsec_ids)	id_node_flow;
-	struct ipsec_id		*id_local;
-	struct ipsec_id		*id_remote;
-	u_int32_t		id_flow;
-	int			id_refcount;
-	u_int			id_gc_ttl;
+	LIST_ENTRY(ipsec_ids)	id_gc_list;	/* [F] */
+	RBT_ENTRY(ipsec_ids)	id_node_id;	/* [F] */
+	RBT_ENTRY(ipsec_ids)	id_node_flow;	/* [F] */
+	struct ipsec_id		*id_local;	/* [I] */
+	struct ipsec_id		*id_remote;	/* [I] */
+	u_int32_t		id_flow;	/* [I] */
+	u_int			id_refcount;	/* [a] */
+	u_int			id_gc_ttl;	/* [F] */
 };
 RBT_HEAD(ipsec_ids_flows, ipsec_ids);
 RBT_HEAD(ipsec_ids_tree, ipsec_ids);
@@ -246,10 +254,6 @@ struct ipsec_acquire {
 	TAILQ_ENTRY(ipsec_acquire)	ipa_next;
 };
 
-/*
- * Locks used to protect struct members in this file:
- *	p	ipo_tdb_mtx		link policy to TDB global mutex
- */
 struct ipsec_policy {
 	struct radix_node	ipo_nodes[2];	/* radix tree glue */
 	struct sockaddr_encap	ipo_addr;
@@ -662,7 +666,7 @@ int	checkreplaywindow(struct tdb *, u_int64_t, u_int32_t, u_int32_t *, int);
 int	ipsp_process_packet(struct mbuf *, struct tdb *, int, int);
 int	ipsp_process_done(struct mbuf *, struct tdb *);
 int	ipsp_spd_lookup(struct mbuf *, int, int, int, struct tdb *,
-	    struct inpcb *, struct tdb **, u_int32_t);
+	    struct inpcb *, struct tdb **, struct ipsec_ids *);
 int	ipsp_is_unspecified(union sockaddr_union);
 int	ipsp_aux_match(struct tdb *, struct ipsec_ids *,
 	    struct sockaddr_encap *, struct sockaddr_encap *);
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index 7d8353f707d..8da5d844009 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_output.c,v 1.379 2021/12/23 12:21:48 bluhm Exp $	*/
+/*	$OpenBSD: ip_output.c,v 1.380 2022/01/04 06:32:39 yasuoka Exp $	*/
 /*	$NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $	*/
 
 /*
@@ -541,11 +541,15 @@ ip_output_ipsec_lookup(struct mbuf *m, int hlen, struct inpcb *inp,
 	struct m_tag *mtag;
 	struct tdb_ident *tdbi;
 	struct tdb *tdb;
+	struct ipsec_ids *ids = NULL;
 	int error;
 
 	/* Do we have any pending SAs to apply ? */
+	if (ipsecflowinfo)
+		ids = ipsp_ids_lookup(ipsecflowinfo);
 	error = ipsp_spd_lookup(m, AF_INET, hlen, IPSP_DIRECTION_OUT,
-	    NULL, inp, &tdb, ipsecflowinfo);
+	    NULL, inp, &tdb, ids);
+	ipsp_ids_free(ids);
 	if (error || tdb == NULL) {
 		*tdbout = NULL;
 		return error;
diff --git a/sys/netinet/ip_spd.c b/sys/netinet/ip_spd.c
index 35a40afa2ef..e7fe8d66007 100644
--- a/sys/netinet/ip_spd.c
+++ b/sys/netinet/ip_spd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ip_spd.c,v 1.110 2021/12/16 15:38:03 bluhm Exp $ */
+/* $OpenBSD: ip_spd.c,v 1.111 2022/01/04 06:32:39 yasuoka Exp $ */
 /*
  * The author of this code is Angelos D. Keromytis (angelos@cis.upenn.edu)
  *
@@ -153,7 +153,7 @@ spd_table_walk(unsigned int rtableid,
 int
 ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
     struct tdb *tdbp, struct inpcb *inp, struct tdb **tdbout,
-    u_int32_t ipsecflowinfo)
+    struct ipsec_ids *ipsecflowinfo_ids)
 {
 	struct radix_node_head *rnh;
 	struct radix_node *rn;
@@ -397,9 +397,6 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
 			}
 		}
 
-		if (ipsecflowinfo)
-			ids = ipsp_ids_lookup(ipsecflowinfo);
-
 		/* Check that the cached TDB (if present), is appropriate. */
 		mtx_enter(&ipo_tdb_mtx);
 		if (ipo->ipo_tdb != NULL) {
@@ -411,7 +408,7 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
 				goto nomatchout;
 
 			if (!ipsp_aux_match(ipo->ipo_tdb,
-			    ids ? ids : ipo->ipo_ids,
+			    ipsecflowinfo_ids? ipsecflowinfo_ids: ipo->ipo_ids,
 			    &ipo->ipo_addr, &ipo->ipo_mask))
 				goto nomatchout;
 
@@ -450,8 +447,10 @@ ipsp_spd_lookup(struct mbuf *m, int af, int hlen, int direction,
 			/* Find an appropriate SA from the existing ones. */
 			tdbp_new = gettdbbydst(rdomain,
 			    dignore ? &sdst : &ipo->ipo_dst,
-			    ipo->ipo_sproto, ids ? ids: ipo->ipo_ids,
+			    ipo->ipo_sproto,
+			    ipsecflowinfo_ids? ipsecflowinfo_ids: ipo->ipo_ids,
 			    &ipo->ipo_addr, &ipo->ipo_mask);
+			ids = NULL;
 			mtx_enter(&ipo_tdb_mtx);
 			if ((tdbp_new != NULL) &&
 			    (tdbp_new->tdb_flags & TDBF_DELETED)) {
diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c
index ee7d0475122..93eafc62de3 100644
--- a/sys/netinet/ipsec_input.c
+++ b/sys/netinet/ipsec_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ipsec_input.c,v 1.201 2021/12/23 12:21:48 bluhm Exp $	*/
+/*	$OpenBSD: ipsec_input.c,v 1.202 2022/01/04 06:32:39 yasuoka Exp $	*/
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -1023,7 +1023,7 @@ ipsec_forward_check(struct mbuf *m, int hlen, int af)
 	} else
 		tdb = NULL;
 	error = ipsp_spd_lookup(m, af, hlen, IPSP_DIRECTION_IN,
-	    tdb, NULL, NULL, 0);
+	    tdb, NULL, NULL, NULL);
 	tdb_unref(tdb);
 
 	return error;
@@ -1096,7 +1096,7 @@ ipsec_local_check(struct mbuf *m, int hlen, int proto, int af)
 	} else
 		tdb = NULL;
 	error = ipsp_spd_lookup(m, af, hlen, IPSP_DIRECTION_IN,
-	    tdb, NULL, NULL, 0);
+	    tdb, NULL, NULL, NULL);
 	tdb_unref(tdb);
 
 	return error;
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 66894e2b63a..c4f22ccf784 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_input.c,v 1.374 2022/01/02 22:36:04 jsg Exp $	*/
+/*	$OpenBSD: tcp_input.c,v 1.375 2022/01/04 06:32:39 yasuoka Exp $	*/
 /*	$NetBSD: tcp_input.c,v 1.23 1996/02/13 23:43:44 christos Exp $	*/
 
 /*
@@ -579,7 +579,7 @@ findpcb:
 			    &tdbi->dst, tdbi->proto);
 		}
 		error = ipsp_spd_lookup(m, af, iphlen, IPSP_DIRECTION_IN,
-		    tdb, inp, NULL, 0);
+		    tdb, inp, NULL, NULL);
 		tdb_unref(tdb);
 		if (error) {
 			tcpstat_inc(tcps_rcvnosec);
diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index f530377c914..742ba1a7fd3 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: udp_usrreq.c,v 1.267 2021/12/02 12:39:15 bluhm Exp $	*/
+/*	$OpenBSD: udp_usrreq.c,v 1.268 2022/01/04 06:32:40 yasuoka Exp $	*/
 /*	$NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $	*/
 
 /*
@@ -512,7 +512,7 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
 		} else
 			tdb = NULL;
 		error = ipsp_spd_lookup(m, af, iphlen, IPSP_DIRECTION_IN,
-		    tdb, inp, NULL, 0);
+		    tdb, inp, NULL, NULL);
 		if (error) {
 			udpstat_inc(udps_nosec);
 			tdb_unref(tdb);
diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c
index ba3e076b474..f9bfa9b5000 100644
--- a/sys/netinet6/ip6_output.c
+++ b/sys/netinet6/ip6_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_output.c,v 1.266 2022/01/02 22:36:04 jsg Exp $	*/
+/*	$OpenBSD: ip6_output.c,v 1.267 2022/01/04 06:32:40 yasuoka Exp $	*/
 /*	$KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $	*/
 
 /*
@@ -2757,7 +2757,7 @@ ip6_output_ipsec_lookup(struct mbuf *m, struct inpcb *inp, struct tdb **tdbout)
 
 	/* Do we have any pending SAs to apply ? */
 	error = ipsp_spd_lookup(m, AF_INET6, sizeof(struct ip6_hdr),
-	    IPSP_DIRECTION_OUT, NULL, inp, &tdb, 0);
+	    IPSP_DIRECTION_OUT, NULL, inp, &tdb, NULL);
 	if (error || tdb == NULL) {
 		*tdbout = NULL;
 		return error;