git: 3d2041c0353d - main - raw ip: merge rip_output() into rip_send()

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Thu, 11 Aug 2022 16:20:17 UTC
The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=3d2041c0353d3cc44bd2a6e37bf1c6e341d2b4db

commit 3d2041c0353d3cc44bd2a6e37bf1c6e341d2b4db
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2022-08-11 16:19:37 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2022-08-11 16:19:37 +0000

    raw ip: merge rip_output() into rip_send()
    
    While here, address the unlocked 'dst' read.  Solve that by storing
    a pointer either to the inpcb or to the sockaddr.  If we end up
    copying address out of the inpcb, that would be done under the read
    lock section.
    
    Reviewed by:            melifaro
    Differential revision:  https://reviews.freebsd.org/D36127
---
 sys/netinet/ip_var.h |  1 -
 sys/netinet/raw_ip.c | 97 ++++++++++++++++++++++------------------------------
 2 files changed, 40 insertions(+), 58 deletions(-)

diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index 4eaaef5c6991..92447c519cc3 100644
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -236,7 +236,6 @@ void	ip_fillid(struct ip *);
 int	rip_ctloutput(struct socket *, struct sockopt *);
 void	rip_ctlinput(int, struct sockaddr *, void *);
 int	rip_input(struct mbuf **, int *, int);
-int	rip_output(struct mbuf *, struct socket *, ...);
 int	ipip_input(struct mbuf **, int *, int);
 int	rsvp_input(struct mbuf **, int *, int);
 int	ip_rsvp_init(struct socket *);
diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c
index 0bd874c717e6..bda0138107ae 100644
--- a/sys/netinet/raw_ip.c
+++ b/sys/netinet/raw_ip.c
@@ -406,23 +406,50 @@ rip_input(struct mbuf **mp, int *offp, int proto)
  * Generate IP header and pass packet to ip_output.  Tack on options user may
  * have setup with control call.
  */
-int
-rip_output(struct mbuf *m, struct socket *so, ...)
+static int
+rip_send(struct socket *so, int pruflags, struct mbuf *m, struct sockaddr *nam,
+    struct mbuf *control, struct thread *td)
 {
 	struct epoch_tracker et;
 	struct ip *ip;
-	int error;
-	struct inpcb *inp = sotoinpcb(so);
-	va_list ap;
-	u_long dst;
-	int flags = ((so->so_options & SO_DONTROUTE) ? IP_ROUTETOIF : 0) |
-	    IP_ALLOWBROADCAST;
-	int cnt, hlen;
+	struct inpcb *inp;
+	in_addr_t *dst;
+	int error, flags, cnt, hlen;
 	u_char opttype, optlen, *cp;
 
-	va_start(ap, so);
-	dst = va_arg(ap, u_long);
-	va_end(ap);
+	inp = sotoinpcb(so);
+	KASSERT(inp != NULL, ("rip_send: inp == NULL"));
+
+	if (control != NULL) {
+		m_freem(control);
+		control = NULL;
+	}
+
+	if (so->so_state & SS_ISCONNECTED) {
+		if (nam) {
+			error = EISCONN;
+			m_freem(m);
+			return (error);
+		}
+		dst = &inp->inp_faddr.s_addr;
+	} else {
+		if (nam == NULL)
+			error = ENOTCONN;
+		else if (nam->sa_family != AF_INET)
+			error = EAFNOSUPPORT;
+		else if (nam->sa_len != sizeof(struct sockaddr_in))
+			error = EINVAL;
+		else
+			error = 0;
+		if (error != 0) {
+			m_freem(m);
+			return (error);
+		}
+		dst = &((struct sockaddr_in *)nam)->sin_addr.s_addr;
+	}
+
+	flags = ((so->so_options & SO_DONTROUTE) ? IP_ROUTETOIF : 0) |
+	    IP_ALLOWBROADCAST;
 
 	/*
 	 * If the user handed us a complete IP packet, use it.  Otherwise,
@@ -447,7 +474,7 @@ rip_output(struct mbuf *m, struct socket *so, ...)
 		ip->ip_p = inp->inp_ip_p;
 		ip->ip_len = htons(m->m_pkthdr.len);
 		ip->ip_src = inp->inp_laddr;
-		ip->ip_dst.s_addr = dst;
+		ip->ip_dst.s_addr = *dst;
 #ifdef ROUTE_MPATH
 		if (CALC_FLOWID_OUTBOUND) {
 			uint32_t hash_type, hash_val;
@@ -971,50 +998,6 @@ rip_shutdown(struct socket *so)
 	INP_WUNLOCK(inp);
 	return (0);
 }
-
-static int
-rip_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
-    struct mbuf *control, struct thread *td)
-{
-	struct inpcb *inp;
-	u_long dst;
-	int error;
-
-	inp = sotoinpcb(so);
-	KASSERT(inp != NULL, ("rip_send: inp == NULL"));
-
-	if (control != NULL) {
-		m_freem(control);
-		control = NULL;
-	}
-
-	/*
-	 * Note: 'dst' reads below are unlocked.
-	 */
-	if (so->so_state & SS_ISCONNECTED) {
-		if (nam) {
-			error = EISCONN;
-			goto release;
-		}
-		dst = inp->inp_faddr.s_addr;	/* Unlocked read. */
-	} else {
-		error = 0;
-		if (nam == NULL)
-			error = ENOTCONN;
-		else if (nam->sa_family != AF_INET)
-			error = EAFNOSUPPORT;
-		else if (nam->sa_len != sizeof(struct sockaddr_in))
-			error = EINVAL;
-		if (error != 0)
-			goto release;
-		dst = ((struct sockaddr_in *)nam)->sin_addr.s_addr;
-	}
-	return (rip_output(m, so, dst));
-
-release:
-	m_freem(m);
-	return (error);
-}
 #endif /* INET */
 
 static int