From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Switch AF_KEY sockets to the new locking scheme
To: tech@openbsd.org
Date: Wed, 8 May 2024 13:52:21 +0300

The simplest case. Nothing to change in sockets layer, only set
SB_MTXLOCK on buffers. This diff doesn't conflict with "Turn sblock()
to `sb_lock' ..." [1] diff.

1. https://marc.info/?t=171478067200001&r=1&w=2

Not related to this diff, but pkpcb uses solock() only to protect
`kcb_flags' and `kcb_reg'. Protected paths are pretty small and have no
context switch, so per-pkpcb mutex(9) could be used instead of solock().
This removes context switch from `kcb_list' walkthroughs and this SRP
could be converted to SMR list.

Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
diff -u -p -r1.333 uipc_socket.c
--- sys/kern/uipc_socket.c	3 May 2024 17:43:09 -0000	1.333
+++ sys/kern/uipc_socket.c	8 May 2024 10:43:14 -0000
@@ -166,6 +166,7 @@ soalloc(const struct protosw *prp, int w
 			break;
 		}
 		break;
+	case AF_KEY:
 	case AF_UNIX:
 		so->so_snd.sb_flags |= SB_MTXLOCK;
 		so->so_rcv.sb_flags |= SB_MTXLOCK;
Index: sys/net/pfkeyv2.c
===================================================================
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
diff -u -p -r1.260 pfkeyv2.c
--- sys/net/pfkeyv2.c	11 Jan 2024 14:15:11 -0000	1.260
+++ sys/net/pfkeyv2.c	8 May 2024 10:43:14 -0000
@@ -443,8 +443,7 @@ pfkey_sendup(struct pkpcb *kp, struct mb
 {
 	struct socket *so = kp->kcb_socket;
 	struct mbuf *m;
-
-	soassertlocked(so);
+	int ret;
 
 	if (more) {
 		if (!(m = m_dup_pkt(m0, 0, M_DONTWAIT)))
@@ -452,7 +451,11 @@ pfkey_sendup(struct pkpcb *kp, struct mb
 	} else
 		m = m0;
 
-	if (!sbappendaddr(so, &so->so_rcv, &pfkey_addr, m, NULL)) {
+	mtx_enter(&so->so_rcv.sb_mtx);
+	ret = sbappendaddr(so, &so->so_rcv, &pfkey_addr, m, NULL);
+	mtx_leave(&so->so_rcv.sb_mtx);
+
+	if (ret == 0) {
 		m_freem(m);
 		return (ENOBUFS);
 	}
@@ -515,9 +518,7 @@ pfkeyv2_sendmessage(void **headers, int 
 		 * Send message to the specified socket, plus all
 		 * promiscuous listeners.
 		 */
-		solock(so);
 		pfkey_sendup(sotokeycb(so), packet, 0);
-		sounlock(so);
 
 		/*
 		 * Promiscuous messages contain the original message
@@ -544,10 +545,8 @@ pfkeyv2_sendmessage(void **headers, int 
 			if (kp->kcb_socket == so || kp->kcb_rdomain != rdomain)
 				continue;
 
-			keylock(kp);
 			if (kp->kcb_flags & PFKEYV2_SOCKETFLAGS_PROMISC)
 				pfkey_sendup(kp, packet, 1);
-			keyunlock(kp);
 		}
 		SRPL_LEAVE(&sr);
 		m_freem(packet);
@@ -562,18 +561,18 @@ pfkeyv2_sendmessage(void **headers, int 
 			if (kp->kcb_rdomain != rdomain)
 				continue;
 
-			keylock(kp);
 			if (kp->kcb_flags & PFKEYV2_SOCKETFLAGS_REGISTERED) {
 				if (!satype) {
 					/* Just send to everyone registered */
 					pfkey_sendup(kp, packet, 1);
 				} else {
+					keylock(kp);
 					/* Check for specified satype */
 					if ((1 << satype) & kp->kcb_reg)
 						pfkey_sendup(kp, packet, 1);
+					keyunlock(kp);
 				}
 			}
-			keyunlock(kp);
 		}
 		SRPL_LEAVE(&sr);
 		/* Free last/original copy of the packet */
@@ -595,14 +594,14 @@ pfkeyv2_sendmessage(void **headers, int 
 
 		/* Send to all registered promiscuous listeners */
 		SRPL_FOREACH(kp, &sr, &pkptable.pkp_list, kcb_list) {
+			int flags = READ_ONCE(kp->kcb_flags);
+
 			if (kp->kcb_rdomain != rdomain)
 				continue;
 
-			keylock(kp);
-			if ((kp->kcb_flags & PFKEYV2_SOCKETFLAGS_PROMISC) &&
-			    !(kp->kcb_flags & PFKEYV2_SOCKETFLAGS_REGISTERED))
+			if ((flags & PFKEYV2_SOCKETFLAGS_PROMISC) &&
+			    !(flags & PFKEYV2_SOCKETFLAGS_REGISTERED))
 				pfkey_sendup(kp, packet, 1);
-			keyunlock(kp);
 		}
 		SRPL_LEAVE(&sr);
 		m_freem(packet);
@@ -614,9 +613,7 @@ pfkeyv2_sendmessage(void **headers, int 
 			if (kp->kcb_rdomain != rdomain)
 				continue;
 
-			keylock(kp);
 			pfkey_sendup(kp, packet, 1);
-			keyunlock(kp);
 		}
 		SRPL_LEAVE(&sr);
 		m_freem(packet);
@@ -1196,10 +1193,8 @@ pfkeyv2_dosend(struct socket *so, void *
 			if (bkp->kcb_rdomain != kp->kcb_rdomain)
 				continue;
 
-			keylock(bkp);
 			if (bkp->kcb_flags & PFKEYV2_SOCKETFLAGS_PROMISC)
 				pfkey_sendup(bkp, packet, 1);
-			keyunlock(bkp);
 		}
 		SRPL_LEAVE(&sr);
 
@@ -2049,14 +2044,13 @@ pfkeyv2_dosend(struct socket *so, void *
 				goto ret;
 
 			SRPL_FOREACH(bkp, &sr, &pkptable.pkp_list, kcb_list) {
-				if (bkp == kp || bkp->kcb_rdomain != kp->kcb_rdomain)
+				if (bkp == kp ||
+				    bkp->kcb_rdomain != kp->kcb_rdomain)
 					continue;
 
 				if (!smsg->sadb_msg_seq ||
 				    (smsg->sadb_msg_seq == kp->kcb_pid)) {
-					keylock(bkp);
 					pfkey_sendup(bkp, packet, 1);
-					keyunlock(bkp);
 				}
 			}
 			SRPL_LEAVE(&sr);