To make protocol input functions MP safe, internet PCB need protection.

Use their reference counter in more places.
The in_pcb lookup functions hold the PCBs in hash tables protected
by table->inpt_mtx mutex.  Whenever a result is returned, increment
the ref count before releasing the mutex.  Then the inp can be used
as long as neccessary.  Unref it at the end of all functions that
call in_pcb lookup.
As a shortcut, pf may also hold a reference to the PCB.  When
pf_inp_lookup() returns it, it also incements the ref count and the
caller can handle it like the inp from table lookup.
OK sashan@

5 files changed, 98 insertions, 47 deletions

diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index f55eaf3e3fe..e5700bb1849 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in_pcb.c,v 1.269 2022/08/06 15:57:59 bluhm Exp $	*/
+/*	$OpenBSD: in_pcb.c,v 1.270 2022/08/08 12:06:30 bluhm Exp $	*/
 /*	$NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $	*/
 
 /*
@@ -403,17 +403,24 @@ in_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in *sin, int wild,
 	}
 	if (lport) {
 		struct inpcb *t;
+		int error = 0;
 
 		if (so->so_euid && !IN_MULTICAST(sin->sin_addr.s_addr)) {
 			t = in_pcblookup_local(table, &sin->sin_addr, lport,
 			    INPLOOKUP_WILDCARD, inp->inp_rtableid);
 			if (t && (so->so_euid != t->inp_socket->so_euid))
-				return (EADDRINUSE);
+				error = EADDRINUSE;
+			in_pcbunref(t);
+			if (error)
+				return (error);
 		}
 		t = in_pcblookup_local(table, &sin->sin_addr, lport,
 		    wild, inp->inp_rtableid);
 		if (t && (reuseport & t->inp_socket->so_options) == 0)
-			return (EADDRINUSE);
+			error = EADDRINUSE;
+		in_pcbunref(t);
+		if (error)
+			return (error);
 	}
 
 	return (0);
@@ -425,6 +432,7 @@ in_pcbpickport(u_int16_t *lport, void *laddr, int wild, struct inpcb *inp,
 {
 	struct socket *so = inp->inp_socket;
 	struct inpcbtable *table = inp->inp_table;
+	struct inpcb *t;
 	u_int16_t first, last, lower, higher, candidate, localport;
 	int count;
 
@@ -456,16 +464,20 @@ in_pcbpickport(u_int16_t *lport, void *laddr, int wild, struct inpcb *inp,
 	count = higher - lower;
 	candidate = lower + arc4random_uniform(count);
 
+	t = NULL;
 	do {
-		if (count-- < 0)	/* completely used? */
-			return (EADDRNOTAVAIL);
-		++candidate;
-		if (candidate < lower || candidate > higher)
-			candidate = lower;
-		localport = htons(candidate);
-	} while (in_baddynamic(candidate, so->so_proto->pr_protocol) ||
-	    in_pcblookup_local(table, laddr, localport, wild,
-	    inp->inp_rtableid));
+		in_pcbunref(t);
+		do {
+			if (count-- < 0)	/* completely used? */
+				return (EADDRNOTAVAIL);
+			++candidate;
+			if (candidate < lower || candidate > higher)
+				candidate = lower;
+			localport = htons(candidate);
+		} while (in_baddynamic(candidate, so->so_proto->pr_protocol));
+		t = in_pcblookup_local(table, laddr, localport, wild,
+		    inp->inp_rtableid);
+	} while (t != NULL);
 	*lport = localport;
 
 	return (0);
@@ -482,6 +494,7 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 {
 	struct in_addr ina;
 	struct sockaddr_in *sin;
+	struct inpcb *t;
 	int error;
 
 #ifdef INET6
@@ -498,9 +511,12 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 	if (error)
 		return (error);
 
-	if (in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
-	    ina, inp->inp_lport, inp->inp_rtableid) != NULL)
+	t = in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
+	    ina, inp->inp_lport, inp->inp_rtableid);
+	if (t != NULL) {
+		in_pcbunref(t);
 		return (EADDRINUSE);
+	}
 
 	KASSERT(inp->inp_laddr.s_addr == INADDR_ANY || inp->inp_lport);
 
@@ -509,10 +525,12 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 			error = in_pcbbind(inp, NULL, curproc);
 			if (error)
 				return (error);
-			if (in_pcbhashlookup(inp->inp_table, sin->sin_addr,
+			t = in_pcbhashlookup(inp->inp_table, sin->sin_addr,
 			    sin->sin_port, ina, inp->inp_lport,
-			    inp->inp_rtableid) != NULL) {
+			    inp->inp_rtableid);
+			if (t != NULL) {
 				inp->inp_lport = 0;
+				in_pcbunref(t);
 				return (EADDRINUSE);
 			}
 		}
@@ -603,23 +621,26 @@ in_pcbdetach(struct inpcb *inp)
 struct inpcb *
 in_pcbref(struct inpcb *inp)
 {
-	if (inp != NULL)
-		refcnt_take(&inp->inp_refcnt);
+	if (inp == NULL)
+		return NULL;
+	refcnt_take(&inp->inp_refcnt);
 	return inp;
 }
 
 void
 in_pcbunref(struct inpcb *inp)
 {
-	if (refcnt_rele(&inp->inp_refcnt)) {
-		KASSERT((LIST_NEXT(inp, inp_hash) == NULL) ||
-		    (LIST_NEXT(inp, inp_hash) == _Q_INVALID));
-		KASSERT((LIST_NEXT(inp, inp_lhash) == NULL) ||
-		    (LIST_NEXT(inp, inp_lhash) == _Q_INVALID));
-		KASSERT((TAILQ_NEXT(inp, inp_queue) == NULL) ||
-		    (TAILQ_NEXT(inp, inp_queue) == _Q_INVALID));
-		pool_put(&inpcb_pool, inp);
-	}
+	if (inp == NULL)
+		return;
+	if (refcnt_rele(&inp->inp_refcnt) == 0)
+		return;
+	KASSERT((LIST_NEXT(inp, inp_hash) == NULL) ||
+	    (LIST_NEXT(inp, inp_hash) == _Q_INVALID));
+	KASSERT((LIST_NEXT(inp, inp_lhash) == NULL) ||
+	    (LIST_NEXT(inp, inp_lhash) == _Q_INVALID));
+	KASSERT((TAILQ_NEXT(inp, inp_queue) == NULL) ||
+	    (TAILQ_NEXT(inp, inp_queue) == _Q_INVALID));
+	pool_put(&inpcb_pool, inp);
 }
 
 void
@@ -830,6 +851,7 @@ in_pcblookup_local(struct inpcbtable *table, void *laddrp, u_int lport_arg,
 				break;
 		}
 	}
+	in_pcbref(match);
 	mtx_leave(&table->inpt_mtx);
 
 	return (match);
@@ -1119,6 +1141,7 @@ in_pcbhashlookup(struct inpcbtable *table, struct in_addr faddr,
 			break;
 		}
 	}
+	in_pcbref(inp);
 	mtx_leave(&table->inpt_mtx);
 #ifdef DIAGNOSTIC
 	if (inp == NULL && in_pcbnotifymiss) {
@@ -1218,6 +1241,7 @@ in_pcblookup_listen(struct inpcbtable *table, struct in_addr laddr,
 		LIST_REMOVE(inp, inp_hash);
 		LIST_INSERT_HEAD(head, inp, inp_hash);
 	}
+	in_pcbref(inp);
 	mtx_leave(&table->inpt_mtx);
 #ifdef DIAGNOSTIC
 	if (inp == NULL && in_pcbnotifymiss) {
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index c4f22ccf784..b5c9be1f990 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_input.c,v 1.375 2022/01/04 06:32:39 yasuoka Exp $	*/
+/*	$OpenBSD: tcp_input.c,v 1.376 2022/08/08 12:06:30 bluhm Exp $	*/
 /*	$NetBSD: tcp_input.c,v 1.23 1996/02/13 23:43:44 christos Exp $	*/
 
 /*
@@ -723,7 +723,8 @@ findpcb:
 					 * full-blown connection.
 					 */
 					tp = NULL;
-					inp = sotoinpcb(so);
+					in_pcbunref(inp);
+					inp = in_pcbref(sotoinpcb(so));
 					tp = intotcpcb(inp);
 					if (tp == NULL)
 						goto badsyn;	/*XXX*/
@@ -832,6 +833,7 @@ findpcb:
 					tcpstat_inc(tcps_dropsyn);
 					goto drop;
 				}
+				in_pcbunref(inp);
 				return IPPROTO_DONE;
 			}
 		}
@@ -1002,6 +1004,7 @@ findpcb:
 				if (so->so_snd.sb_cc ||
 				    tp->t_flags & TF_NEEDOUTPUT)
 					(void) tcp_output(tp);
+				in_pcbunref(inp);
 				return IPPROTO_DONE;
 			}
 		} else if (th->th_ack == tp->snd_una &&
@@ -1050,6 +1053,7 @@ findpcb:
 			tp->t_flags &= ~TF_BLOCKOUTPUT;
 			if (tp->t_flags & (TF_ACKNOW|TF_NEEDOUTPUT))
 				(void) tcp_output(tp);
+			in_pcbunref(inp);
 			return IPPROTO_DONE;
 		}
 	}
@@ -1244,6 +1248,7 @@ trimthenstep6:
 			    ((arc4random() & 0x7fffffff) | 0x8000);
 			reuse = &iss;
 			tp = tcp_close(tp);
+			in_pcbunref(inp);
 			inp = NULL;
 			goto findpcb;
 		}
@@ -2028,6 +2033,7 @@ dodata:							/* XXX */
 	 */
 	if (tp->t_flags & (TF_ACKNOW|TF_NEEDOUTPUT))
 		(void) tcp_output(tp);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 
 badsyn:
@@ -2056,6 +2062,7 @@ dropafterack:
 	m_freem(m);
 	tp->t_flags |= TF_ACKNOW;
 	(void) tcp_output(tp);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 
 dropwithreset_ratelim:
@@ -2090,6 +2097,7 @@ dropwithreset:
 		    (tcp_seq)0, TH_RST|TH_ACK, m->m_pkthdr.ph_rtableid);
 	}
 	m_freem(m);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 
 drop:
@@ -2100,6 +2108,7 @@ drop:
 		tcp_trace(TA_DROP, ostate, tp, otp, saveti, 0, tlen);
 
 	m_freem(m);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 }
 
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index bd0671b319a..b7d5eb22dec 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_subr.c,v 1.184 2022/03/02 12:53:15 bluhm Exp $	*/
+/*	$OpenBSD: tcp_subr.c,v 1.185 2022/08/08 12:06:30 bluhm Exp $	*/
 /*	$NetBSD: tcp_subr.c,v 1.22 1996/02/13 23:44:00 christos Exp $	*/
 
 /*
@@ -671,7 +671,9 @@ tcp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 			 *   corresponding routing entry, or
 			 * - ignore the MTU change notification.
 			 */
-			icmp6_mtudisc_update((struct ip6ctlparam *)d, inp != NULL);
+			icmp6_mtudisc_update((struct ip6ctlparam *)d,
+			    inp != NULL);
+			in_pcbunref(inp);
 			return;
 		}
 		if (inp) {
@@ -686,6 +688,7 @@ tcp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 		    inet6ctlerrmap[cmd] == EHOSTDOWN)
 			syn_cache_unreach((struct sockaddr *)sa6_src,
 			    sa, &th, rdomain);
+		in_pcbunref(inp);
 	} else {
 		in6_pcbnotify(&tcbtable, sa6, 0,
 		    sa6_src, 0, rdomain, cmd, NULL, notify);
@@ -746,8 +749,10 @@ tcp_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *v)
 			 * ever sent, drop the message.
 			 */
 			mtu = (u_int)ntohs(icp->icmp_nextmtu);
-			if (mtu >= tp->t_pmtud_mtu_sent)
+			if (mtu >= tp->t_pmtud_mtu_sent) {
+				in_pcbunref(inp);
 				return;
+			}
 			if (mtu >= tcp_hdrsz(tp) + tp->t_pmtud_mss_acked) {
 				/* 
 				 * Calculate new MTU, and create corresponding
@@ -764,20 +769,25 @@ tcp_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *v)
 				 * refers to an older TCP segment
 				 */
 				if (tp->t_flags & TF_PMTUD_PEND) {
-					if (SEQ_LT(tp->t_pmtud_th_seq, seq))
+					if (SEQ_LT(tp->t_pmtud_th_seq, seq)) {
+						in_pcbunref(inp);
 						return;
+					}
 				} else
 					tp->t_flags |= TF_PMTUD_PEND;
 				tp->t_pmtud_th_seq = seq;
 				tp->t_pmtud_nextmtu = icp->icmp_nextmtu;
 				tp->t_pmtud_ip_len = icp->icmp_ip.ip_len;
 				tp->t_pmtud_ip_hl = icp->icmp_ip.ip_hl;
+				in_pcbunref(inp);
 				return;
 			}
 		} else {
 			/* ignore if we don't have a matching connection */
+			in_pcbunref(inp);
 			return;
 		}
+		in_pcbunref(inp);
 		notify = tcp_mtudisc, ip = 0;
 	} else if (cmd == PRC_MTUINC)
 		notify = tcp_mtudisc_increase, ip = 0;
@@ -810,6 +820,7 @@ tcp_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *v)
 			sin.sin_addr = ip->ip_src;
 			syn_cache_unreach(sintosa(&sin), sa, th, rdomain);
 		}
+		in_pcbunref(inp);
 	} else
 		in_pcbnotifyall(&tcbtable, sa, rdomain, errno, notify);
 }
diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c
index 67035bd4553..6185187ca0b 100644
--- a/sys/netinet/tcp_usrreq.c
+++ b/sys/netinet/tcp_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_usrreq.c,v 1.183 2022/02/25 23:51:03 guenther Exp $	*/
+/*	$OpenBSD: tcp_usrreq.c,v 1.184 2022/08/08 12:06:30 bluhm Exp $	*/
 /*	$NetBSD: tcp_usrreq.c,v 1.20 1996/02/13 23:44:16 christos Exp $	*/
 
 /*
@@ -822,6 +822,7 @@ tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop)
 			tp = tcp_drop(tp, ECONNABORTED);
 		else
 			error = ESRCH;
+		in_pcbunref(inp);
 		return (error);
 	}
 
@@ -851,6 +852,7 @@ tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop)
 
 	*oldlenp = sizeof (tir);
 	error = copyout((void *)&tir, oldp, sizeof (tir));
+	in_pcbunref(inp);
 	return (error);
 }
 
diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index 480cbcc7818..bf10be68923 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: udp_usrreq.c,v 1.280 2022/08/06 15:57:59 bluhm Exp $	*/
+/*	$OpenBSD: udp_usrreq.c,v 1.281 2022/08/08 12:06:30 bluhm Exp $	*/
 /*	$NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $	*/
 
 /*
@@ -570,17 +570,20 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
 			m = *mp = pipex_l2tp_input(m, off, session,
 			    ipsecflowinfo);
 			pipex_rele_session(session);
-
-			if (m == NULL)
+			if (m == NULL) {
+				in_pcbunref(inp);
 				return IPPROTO_DONE;
+			}
 		}
 	}
 #endif
 
 	udp_sbappend(inp, m, ip, ip6, iphlen, uh, &srcsa.sa, ipsecflowinfo);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 bad:
 	m_freem(m);
+	in_pcbunref(inp);
 	return IPPROTO_DONE;
 }
 
@@ -674,6 +677,7 @@ udp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 		u_int16_t uh_sport;
 		u_int16_t uh_dport;
 	} *uhp;
+	struct inpcb *inp;
 	void (*notify)(struct inpcb *, int) = udp_notify;
 
 	if (sa == NULL)
@@ -759,17 +763,14 @@ udp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 		}
 
 		if (cmd == PRC_MSGSIZE) {
-			int valid = 0;
-
 			/*
 			 * Check to see if we have a valid UDP socket
 			 * corresponding to the address in the ICMPv6 message
 			 * payload.
 			 */
-			if (in6_pcbhashlookup(&udbtable, &sa6.sin6_addr,
+			inp = in6_pcbhashlookup(&udbtable, &sa6.sin6_addr,
 			    uh.uh_dport, &sa6_src.sin6_addr, uh.uh_sport,
-			    rdomain))
-				valid = 1;
+			    rdomain);
 #if 0
 			/*
 			 * As the use of sendto(2) is fairly popular,
@@ -778,10 +779,11 @@ udp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 			 * We should at least check if the local address (= s)
 			 * is really ours.
 			 */
-			else if (in6_pcblookup_listen(&udbtable,
-			    &sa6_src.sin6_addr, uh.uh_sport, NULL,
-			    rdomain))
-				valid = 1;
+			if (inp == NULL) {
+				inp = in6_pcblookup_listen(&udbtable,
+				    &sa6_src.sin6_addr, uh.uh_sport, NULL,
+				    rdomain))
+			}
 #endif
 
 			/*
@@ -791,7 +793,9 @@ udp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
 			 *   corresponding routing entry, or
 			 * - ignore the MTU change notification.
 			 */
-			icmp6_mtudisc_update((struct ip6ctlparam *)d, valid);
+			icmp6_mtudisc_update((struct ip6ctlparam *)d,
+			    inp != NULL);
+			in_pcbunref(inp);
 
 			/*
 			 * regardless of if we called icmp6_mtudisc_update(),
@@ -855,6 +859,7 @@ udp_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *v)
 		    rdomain);
 		if (inp && inp->inp_socket != NULL)
 			notify(inp, errno);
+		in_pcbunref(inp);
 	} else
 		in_pcbnotifyall(&udbtable, sa, rdomain, errno, notify);
 }