From: claudio <claudio@openbsd.org>
Date: Mon, 26 Apr 2021 08:21:35 +0000 (+0000)
Subject: Revert per-socket `so_lock' rwlock(9) and use it to protect routing
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=158ae97bf6f3c6848e7347b79ca3f7d3d19902a4;p=openbsd

Revert per-socket `so_lock' rwlock(9) and use it to protect routing
(PF_ROUTE) sockets. There is a locking issue with timeouts that needs
to be fixed.
Requested by deraadt@
---

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index d457f366c09..52b6c474b74 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.257 2021/04/25 00:00:34 mvs Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.258 2021/04/26 08:21:35 claudio Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -151,7 +151,6 @@ socreate(int dom, struct socket **aso, int type, int proto)
 	if (prp->pr_type != type)
 		return (EPROTOTYPE);
 	so = pool_get(&socket_pool, PR_WAITOK | PR_ZERO);
-	rw_init(&so->so_lock, "solock");
 	sigio_init(&so->so_sigio);
 	TAILQ_INIT(&so->so_q0);
 	TAILQ_INIT(&so->so_q);
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index 068e3fb793d..940da149093 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket2.c,v 1.107 2021/04/25 00:00:34 mvs Exp $	*/
+/*	$OpenBSD: uipc_socket2.c,v 1.108 2021/04/26 08:21:35 claudio Exp $	*/
 /*	$NetBSD: uipc_socket2.c,v 1.11 1996/02/04 02:17:55 christos Exp $	*/
 
 /*
@@ -162,7 +162,6 @@ sonewconn(struct socket *head, int connstatus)
 	so = pool_get(&socket_pool, PR_NOWAIT|PR_ZERO);
 	if (so == NULL)
 		return (NULL);
-	rw_init(&so->so_lock, "solock");
 	so->so_type = head->so_type;
 	so->so_options = head->so_options &~ SO_ACCEPTCONN;
 	so->so_linger = head->so_linger;
@@ -288,8 +287,6 @@ solock(struct socket *so)
 		rw_enter_write(&unp_lock);
 		break;
 	case PF_ROUTE:
-		rw_enter_write(&so->so_lock);
-		break;
 	case PF_KEY:
 	default:
 		KERNEL_LOCK();
@@ -316,8 +313,6 @@ sounlock(struct socket *so, int s)
 		rw_exit_write(&unp_lock);
 		break;
 	case PF_ROUTE:
-		rw_exit_write(&so->so_lock);
-		break;
 	case PF_KEY:
 	default:
 		KERNEL_UNLOCK();
@@ -337,8 +332,6 @@ soassertlocked(struct socket *so)
 		rw_assert_wrlock(&unp_lock);
 		break;
 	case PF_ROUTE:
-		rw_assert_wrlock(&so->so_lock);
-		break;
 	case PF_KEY:
 	default:
 		KERNEL_ASSERT_LOCKED();
@@ -361,8 +354,6 @@ sosleep_nsec(struct socket *so, void *ident, int prio, const char *wmesg,
 		ret = rwsleep_nsec(ident, &unp_lock, prio, wmesg, nsecs);
 		break;
 	case PF_ROUTE:
-		ret = rwsleep_nsec(ident, &so->so_lock, prio, wmesg, nsecs);
-		break;
 	case PF_KEY:
 	default:
 		ret = tsleep_nsec(ident, prio, wmesg, nsecs);
diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index ad662ddfa18..ac7c163128e 100644
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rtsock.c,v 1.310 2021/04/25 00:00:35 mvs Exp $	*/
+/*	$OpenBSD: rtsock.c,v 1.311 2021/04/26 08:21:36 claudio Exp $	*/
 /*	$NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $	*/
 
 /*
@@ -143,7 +143,7 @@ int		 rt_setsource(unsigned int, struct sockaddr *);
 /*
  * Locks used to protect struct members
  *       I       immutable after creation
- *       s       solock
+ *       sK      solock (kernel lock)
  */
 struct rtpcb {
 	struct socket		*rop_socket;		/* [I] */
@@ -151,12 +151,12 @@ struct rtpcb {
 	SRPL_ENTRY(rtpcb)	rop_list;
 	struct refcnt		rop_refcnt;
 	struct timeout		rop_timeout;
-	unsigned int		rop_msgfilter;		/* [s] */
-	unsigned int		rop_flagfilter;		/* [s] */
-	unsigned int		rop_flags;		/* [s] */
-	u_int			rop_rtableid;		/* [s] */
+	unsigned int		rop_msgfilter;		/* [sK] */
+	unsigned int		rop_flagfilter;		/* [sK] */
+	unsigned int		rop_flags;		/* [sK] */
+	u_int			rop_rtableid;		/* [sK] */
 	unsigned short		rop_proto;		/* [I] */
-	u_char			rop_priority;		/* [s] */
+	u_char			rop_priority;		/* [sK] */
 };
 #define	sotortpcb(so)	((struct rtpcb *)(so)->so_pcb)
 
@@ -188,7 +188,7 @@ route_prinit(void)
 	rw_init(&rtptable.rtp_lk, "rtsock");
 	SRPL_INIT(&rtptable.rtp_list);
 	pool_init(&rtpcb_pool, sizeof(struct rtpcb), 0,
-	    IPL_SOFTNET, PR_WAITOK, "rtpcb", NULL);
+	    IPL_NONE, PR_WAITOK, "rtpcb", NULL);
 }
 
 void
@@ -352,13 +352,9 @@ route_detach(struct socket *so)
 	    rop_list);
 	rw_exit(&rtptable.rtp_lk);
 
-	sounlock(so, SL_LOCKED);
-
 	/* wait for all references to drop */
 	refcnt_finalize(&rop->rop_refcnt, "rtsockrefs");
 
-	solock(so);
-
 	so->so_pcb = NULL;
 	KASSERT((so->so_state & SS_NOFDREF) == 0);
 	pool_put(&rtpcb_pool, rop);
@@ -680,7 +676,7 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
 	struct rtentry		*rt = NULL;
 	struct rt_addrinfo	 info;
 	struct ifnet		*ifp;
-	int			 len, seq, useloopback, error = 0;
+	int			 len, seq, error = 0;
 	u_int			 tableid;
 	u_int8_t		 prio;
 	u_char			 vers, type;
@@ -690,16 +686,6 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
 		return (ENOBUFS);
 	if ((m->m_flags & M_PKTHDR) == 0)
 		panic("route_output");
-
-	useloopback = so->so_options & SO_USELOOPBACK;
-
-	/*
-	 * The socket can't be closed concurrently because the file
-	 * descriptor reference is still held.
-	 */
-
-	sounlock(so, SL_LOCKED);
-
 	len = m->m_pkthdr.len;
 	if (len < offsetof(struct rt_msghdr, rtm_hdrlen) + 1 ||
 	    len != mtod(m, struct rt_msghdr *)->rtm_msglen) {
@@ -873,10 +859,13 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
 	/*
 	 * Check to see if we don't want our own messages.
 	 */
-	if (!useloopback) {
-		if (rtptable.rtp_count == 0) {
+	if (!(so->so_options & SO_USELOOPBACK)) {
+		if (rtptable.rtp_count <= 1) {
 			/* no other listener and no loopback of messages */
-			goto fail;
+fail:
+			free(rtm, M_RTABLE, len);
+			m_freem(m);
+			return (error);
 		}
 	}
 	if (m_copyback(m, 0, len, rtm, M_NOWAIT)) {
@@ -888,13 +877,6 @@ route_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
 	if (m)
 		route_input(m, so, info.rti_info[RTAX_DST] ?
 		    info.rti_info[RTAX_DST]->sa_family : AF_UNSPEC);
-	solock(so);
-
-	return (error);
-fail:
-	free(rtm, M_RTABLE, len);
-	m_freem(m);
-	solock(so);
 
 	return (error);
 }
diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h
index 6a7dfaafb2a..18845733bcd 100644
--- a/sys/sys/socketvar.h
+++ b/sys/sys/socketvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: socketvar.h,v 1.94 2021/04/25 00:00:35 mvs Exp $	*/
+/*	$OpenBSD: socketvar.h,v 1.95 2021/04/26 08:21:36 claudio Exp $	*/
 /*	$NetBSD: socketvar.h,v 1.18 1996/02/09 18:25:38 christos Exp $	*/
 
 /*-
@@ -37,7 +37,6 @@
 #include <sys/sigio.h>				/* for struct sigio_ref */
 #include <sys/task.h>
 #include <sys/timeout.h>
-#include <sys/rwlock.h>
 
 #ifndef	_SOCKLEN_T_DEFINED_
 #define	_SOCKLEN_T_DEFINED_
@@ -54,7 +53,6 @@ TAILQ_HEAD(soqhead, socket);
  */
 struct socket {
 	const struct protosw *so_proto;	/* protocol handle */
-	struct rwlock so_lock;		/* this socket lock */
 	void	*so_pcb;		/* protocol control block */
 	u_int	so_state;		/* internal state flags SS_*, below */
 	short	so_type;		/* generic type, see socket.h */