From: dlg <dlg@openbsd.org>
Date: Sun, 15 May 2022 09:12:20 +0000 (+0000)
Subject: have in_pcbselsrc copy the selected address to memory provided by the caller.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=089ca45eda6afb3c330881fd7d5a2ef237868262;p=openbsd

have in_pcbselsrc copy the selected address to memory provided by the caller.

having it return a pointer to something that has a lifetime managed
by a lock without accounting for it or taking a reference count or
anything like that is asking for trouble. copying the address to
caller provded memory while still inside the lock is a lot safer.

discussed with visa@
ok bluhm@ claudio@
---

diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
index 9d4749f50e0..a725b171da1 100644
--- a/sys/netinet/in_pcb.c
+++ b/sys/netinet/in_pcb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in_pcb.c,v 1.265 2022/04/14 14:10:22 claudio Exp $	*/
+/*	$OpenBSD: in_pcb.c,v 1.266 2022/05/15 09:12:20 dlg Exp $	*/
 /*	$NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $	*/
 
 /*
@@ -480,7 +480,7 @@ in_pcbpickport(u_int16_t *lport, void *laddr, int wild, struct inpcb *inp,
 int
 in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 {
-	struct in_addr *ina = NULL;
+	struct in_addr ina;
 	struct sockaddr_in *sin;
 	int error;
 
@@ -499,7 +499,7 @@ in_pcbconnect(struct inpcb *inp, struct mbuf *nam)
 		return (error);
 
 	if (in_pcbhashlookup(inp->inp_table, sin->sin_addr, sin->sin_port,
-	    *ina, inp->inp_lport, inp->inp_rtableid) != NULL)
+	    ina, inp->inp_lport, inp->inp_rtableid) != NULL)
 		return (EADDRINUSE);
 
 	KASSERT(inp->inp_laddr.s_addr == INADDR_ANY || inp->inp_lport);
@@ -510,13 +510,13 @@ 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,
+			    sin->sin_port, ina, inp->inp_lport,
 			    inp->inp_rtableid) != NULL) {
 				inp->inp_lport = 0;
 				return (EADDRINUSE);
 			}
 		}
-		inp->inp_laddr = *ina;
+		inp->inp_laddr = ina;
 	}
 	inp->inp_faddr = sin->sin_addr;
 	inp->inp_fport = sin->sin_port;
@@ -893,7 +893,7 @@ in_pcbrtentry(struct inpcb *inp)
  * an entry to the caller for later use.
  */
 int
-in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
+in_pcbselsrc(struct in_addr *insrc, struct sockaddr_in *sin,
     struct inpcb *inp)
 {
 	struct ip_moptions *mopts = inp->inp_moptions;
@@ -909,9 +909,9 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
 	 * If the socket(if any) is already bound, use that bound address
 	 * unless it is INADDR_ANY or INADDR_BROADCAST.
 	 */
-	if (laddr && laddr->s_addr != INADDR_ANY &&
+	if (laddr->s_addr != INADDR_ANY &&
 	    laddr->s_addr != INADDR_BROADCAST) {
-		*insrc = laddr;
+		*insrc = *laddr;
 		return (0);
 	}
 
@@ -934,7 +934,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
 				return (EADDRNOTAVAIL);
 			}
 
-			*insrc = &ia->ia_addr.sin_addr;
+			*insrc = ia->ia_addr.sin_addr;
 			if_put(ifp);
 			return (0);
 		}
@@ -985,7 +985,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
 			struct ifaddr *ifa;
 			if ((ifa = ifa_ifwithaddr(ip4_source, rtableid)) !=
 			    NULL && ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) {
-				*insrc = &satosin(ip4_source)->sin_addr;
+				*insrc = satosin(ip4_source)->sin_addr;
 				return (0);
 			}
 		}
@@ -994,7 +994,7 @@ in_pcbselsrc(struct in_addr **insrc, struct sockaddr_in *sin,
 	if (ia == NULL)
 		return (EADDRNOTAVAIL);
 
-	*insrc = &ia->ia_addr.sin_addr;
+	*insrc = ia->ia_addr.sin_addr;
 	return (0);
 }
 
diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h
index 36ad80d8fbb..c14aa4b1f0d 100644
--- a/sys/netinet/in_pcb.h
+++ b/sys/netinet/in_pcb.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: in_pcb.h,v 1.128 2022/03/21 23:37:09 bluhm Exp $	*/
+/*	$OpenBSD: in_pcb.h,v 1.129 2022/05/15 09:12:20 dlg Exp $	*/
 /*	$NetBSD: in_pcb.h,v 1.14 1996/02/13 23:42:00 christos Exp $	*/
 
 /*
@@ -315,7 +315,7 @@ void	 in_setpeeraddr(struct inpcb *, struct mbuf *);
 void	 in_setsockaddr(struct inpcb *, struct mbuf *);
 int	 in_baddynamic(u_int16_t, u_int16_t);
 int	 in_rootonly(u_int16_t, u_int16_t);
-int	 in_pcbselsrc(struct in_addr **, struct sockaddr_in *, struct inpcb *);
+int	 in_pcbselsrc(struct in_addr *, struct sockaddr_in *, struct inpcb *);
 struct rtentry *
 	in_pcbrtentry(struct inpcb *);
 
diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c
index bb43a7e992a..7e8a062d006 100644
--- a/sys/netinet/raw_ip.c
+++ b/sys/netinet/raw_ip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: raw_ip.c,v 1.127 2022/03/23 17:22:28 bluhm Exp $	*/
+/*	$OpenBSD: raw_ip.c,v 1.128 2022/05/15 09:12:20 dlg Exp $	*/
 /*	$NetBSD: raw_ip.c,v 1.25 1996/02/18 18:58:33 christos Exp $	*/
 
 /*
@@ -276,13 +276,9 @@ rip_output(struct mbuf *m, struct socket *so, struct sockaddr *dstaddr,
 	}
 
 	if (ip->ip_src.s_addr == INADDR_ANY) {
-		struct in_addr *laddr;
-
-		error = in_pcbselsrc(&laddr, dst, inp);
+		error = in_pcbselsrc(&ip->ip_src, dst, inp);
 		if (error != 0)
 			return (error);
-
-		ip->ip_src = *laddr;
 	}
 
 #ifdef INET6
diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c
index a8dc16826ae..84592f9afb9 100644
--- a/sys/netinet/udp_usrreq.c
+++ b/sys/netinet/udp_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: udp_usrreq.c,v 1.277 2022/03/21 23:37:09 bluhm Exp $	*/
+/*	$OpenBSD: udp_usrreq.c,v 1.278 2022/05/15 09:12:20 dlg Exp $	*/
 /*	$NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $	*/
 
 /*
@@ -866,7 +866,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
 	u_int32_t ipsecflowinfo = 0;
 	struct sockaddr_in src_sin;
 	int len = m->m_pkthdr.len;
-	struct in_addr *laddr;
+	struct in_addr laddr;
 	int error = 0;
 
 #ifdef DIAGNOSTIC
@@ -966,14 +966,14 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
 			    (error =
 			    in_pcbaddrisavail(inp, &src_sin, 0, curproc)))
 				goto release;
-			laddr = &src_sin.sin_addr;
+			laddr = src_sin.sin_addr;
 		}
 	} else {
 		if (inp->inp_faddr.s_addr == INADDR_ANY) {
 			error = ENOTCONN;
 			goto release;
 		}
-		laddr = &inp->inp_laddr;
+		laddr = inp->inp_laddr;
 	}
 
 	/*
@@ -994,7 +994,7 @@ udp_output(struct inpcb *inp, struct mbuf *m, struct mbuf *addr,
 	bzero(ui->ui_x1, sizeof ui->ui_x1);
 	ui->ui_pr = IPPROTO_UDP;
 	ui->ui_len = htons((u_int16_t)len + sizeof (struct udphdr));
-	ui->ui_src = *laddr;
+	ui->ui_src = laddr;
 	ui->ui_dst = sin ? sin->sin_addr : inp->inp_faddr;
 	ui->ui_sport = inp->inp_lport;
 	ui->ui_dport = sin ? sin->sin_port : inp->inp_fport;