From efc3101a722ac647497a9d79674ebcb74141053b Mon Sep 17 00:00:00 2001
From: Alexander Bluhm <bluhm@cvs.openbsd.org>
Date: Tue, 22 Mar 2022 18:17:31 +0000
Subject: For raw IP packets rip_input() traverses the loop of all PCBs.  From
 there it calls sbappendaddr() while holding the raw table mutex. This ends in
 sorwakeup() where we finally grab the kernel lock while holding a mutex. 
 Witness detects this misuse. Use the same solution as for PCB notify. 
 Collect the affected PCBs in a temporary list.  The list is protected by
 exclusive net lock. syzbot+ebe3f03a472fecf5e42e@syzkaller.appspotmail.com OK
 claudio@

---
 sys/netinet/raw_ip.c | 70 ++++++++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

(limited to 'sys/netinet')

diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c
index fc7d8dff62e..8b874267217 100644
--- a/sys/netinet/raw_ip.c
+++ b/sys/netinet/raw_ip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: raw_ip.c,v 1.125 2022/03/21 09:12:34 bluhm Exp $	*/
+/*	$OpenBSD: raw_ip.c,v 1.126 2022/03/22 18:17:30 bluhm Exp $	*/
 /*	$NetBSD: raw_ip.c,v 1.25 1996/02/18 18:58:33 christos Exp $	*/
 
 /*
@@ -122,9 +122,9 @@ rip_input(struct mbuf **mp, int *offp, int proto, int af)
 {
 	struct mbuf *m = *mp;
 	struct ip *ip = mtod(m, struct ip *);
-	struct inpcb *inp, *last = NULL;
+	struct inpcb *inp;
+	SIMPLEQ_HEAD(, inpcb) inpcblist;
 	struct in_addr *key;
-	struct mbuf *opts = NULL;
 	struct counters_ref ref;
 	uint64_t *counters;
 
@@ -150,7 +150,8 @@ rip_input(struct mbuf **mp, int *offp, int proto, int af)
 		}
 	}
 #endif
-	NET_ASSERT_LOCKED();
+	NET_ASSERT_WLOCKED();
+	SIMPLEQ_INIT(&inpcblist);
 	mtx_enter(&rawcbtable.inpt_mtx);
 	TAILQ_FOREACH(inp, &rawcbtable.inpt_queue, inp_queue) {
 		if (inp->inp_socket->so_state & SS_CANTRCVMORE)
@@ -171,41 +172,16 @@ rip_input(struct mbuf **mp, int *offp, int proto, int af)
 		if (inp->inp_faddr.s_addr &&
 		    inp->inp_faddr.s_addr != ip->ip_src.s_addr)
 			continue;
-		if (last) {
-			struct mbuf *n;
-
-			if ((n = m_copym(m, 0, M_COPYALL, M_NOWAIT)) != NULL) {
-				if (last->inp_flags & INP_CONTROLOPTS ||
-				    last->inp_socket->so_options & SO_TIMESTAMP)
-					ip_savecontrol(last, &opts, ip, n);
-				if (sbappendaddr(last->inp_socket,
-				    &last->inp_socket->so_rcv,
-				    sintosa(&ripsrc), n, opts) == 0) {
-					/* should notify about lost packet */
-					m_freem(n);
-					m_freem(opts);
-				} else
-					sorwakeup(last->inp_socket);
-				opts = NULL;
-			}
-		}
-		last = inp;
+
+		in_pcbref(inp);
+		SIMPLEQ_INSERT_TAIL(&inpcblist, inp, inp_notify);
 	}
 	mtx_leave(&rawcbtable.inpt_mtx);
 
-	if (last) {
-		if (last->inp_flags & INP_CONTROLOPTS ||
-		    last->inp_socket->so_options & SO_TIMESTAMP)
-			ip_savecontrol(last, &opts, ip, m);
-		if (sbappendaddr(last->inp_socket, &last->inp_socket->so_rcv,
-		    sintosa(&ripsrc), m, opts) == 0) {
-			m_freem(m);
-			m_freem(opts);
-		} else
-			sorwakeup(last->inp_socket);
-	} else {
+	if (SIMPLEQ_EMPTY(&inpcblist)) {
 		if (ip->ip_p != IPPROTO_ICMP)
-			icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PROTOCOL, 0, 0);
+			icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PROTOCOL,
+			    0, 0);
 		else
 			m_freem(m);
 
@@ -214,6 +190,30 @@ rip_input(struct mbuf **mp, int *offp, int proto, int af)
 		counters[ips_delivered]--;
 		counters_leave(&ref, ipcounters);
 	}
+
+	while ((inp = SIMPLEQ_FIRST(&inpcblist)) != NULL) {
+		struct mbuf *n, *opts = NULL;
+
+		SIMPLEQ_REMOVE_HEAD(&inpcblist, inp_notify);
+		if (SIMPLEQ_EMPTY(&inpcblist))
+			n = m;
+		else
+			n = m_copym(m, 0, M_COPYALL, M_NOWAIT);
+		if (n != NULL) {
+			if (inp->inp_flags & INP_CONTROLOPTS ||
+			    inp->inp_socket->so_options & SO_TIMESTAMP)
+				ip_savecontrol(inp, &opts, ip, n);
+			if (sbappendaddr(inp->inp_socket,
+			    &inp->inp_socket->so_rcv,
+			    sintosa(&ripsrc), n, opts) == 0) {
+				/* should notify about lost packet */
+				m_freem(n);
+				m_freem(opts);
+			} else
+				sorwakeup(inp->inp_socket);
+		}
+		in_pcbunref(inp);
+	}
 	return IPPROTO_DONE;
 }
 
-- 
cgit v1.2.3