[PATCH 15/16] tcp, tcp_splice: Better construction of IPv4 or IPv6 sockaddrs

[David Gibson <david@gibson.dropbear.id.au>]

In both tcp_conn_from_tap() and tcp_splice_connect() we need to construct
a socket address for connect() which could be either IPv4 or IPv6.  At the
moment we initialise both a sockaddr_in and a sockaddr_in6 as locals, then
set a pointer to one or the other.  This is a little bit ugly.

More importantly, though, in the case of tcp_conn_from_tap() initialising
the sockaddr_in6 when we're actually passed an IPv4 address will access
memory beyond the implied (struct in_addr) we're passed as daddr.  In
practice that will be a pointer into a packet buffer, so there will be
enough valid memory to get 16 bytes of (garbage) IPv6 address that are then
ignored.  However, it's not a good look to access beyond what the
parameters seem to imply is passed.

We can clean up these cases using sockaddr_inany and a new helper
sockaddr_inany_init().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 inany.h      | 33 +++++++++++++++++++++++++++++++++
 tcp.c        | 37 +++++++++++--------------------------
 tcp_splice.c | 27 ++++++++-------------------
 3 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/inany.h b/inany.h
index 474e09d0..063545b7 100644
--- a/inany.h
+++ b/inany.h
@@ -138,4 +138,37 @@ static inline void inany_siphash_feed(struct siphash_state *state,
 
 const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
 
+/** sockaddr_inany_init - Construct a sockaddr_inany
+ * @sa:		Pointer to sockaddr to fill in
+ * @sl:		Relevant length of @sa after initialisation
+ * @af:		Address family, AF_INET or AF_INET6
+ * @addr:	Address (either in_addr or in6_addr)
+ * @port:	Port (host byte order)
+ * @scope:	Scope ID for AF_INET6 (ignored for AF_INET)
+ */
+static inline void sockaddr_inany_init(union sockaddr_inany *sa, socklen_t *sl,
+				       sa_family_t af, const void *addr,
+				       in_port_t port, uint32_t scope)
+{
+	sa->sa_family = af;
+	switch (af) {
+	case AF_INET:
+		sa->sa4.sin_addr = *(const struct in_addr *)addr;
+		sa->sa4.sin_port = htons(port);
+		*sl = sizeof(sa->sa4);
+		break;
+
+	case AF_INET6:
+		sa->sa6.sin6_addr = *(const struct in6_addr *)addr;
+		sa->sa6.sin6_port = htons(port);
+		sa->sa6.sin6_scope_id = scope;
+		sa->sa6.sin6_flowinfo = 0;
+		*sl = sizeof(sa->sa6);
+		break;
+
+	default:
+		ASSERT(0);
+	}
+}
+
 #endif /* INANY_H */
diff --git a/tcp.c b/tcp.c
index a52a1f84..6c9edbe1 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1930,18 +1930,9 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 			      const struct tcphdr *th, const char *opts,
 			      size_t optlen, const struct timespec *now)
 {
-	struct sockaddr_in addr4 = {
-		.sin_family = AF_INET,
-		.sin_port = th->dest,
-		.sin_addr = *(struct in_addr *)daddr,
-	};
-	struct sockaddr_in6 addr6 = {
-		.sin6_family = AF_INET6,
-		.sin6_port = th->dest,
-		.sin6_addr = *(struct in6_addr *)daddr,
-	};
-	const struct sockaddr *sa;
+	const void *host_daddr = daddr;
 	struct tcp_tap_conn *conn;
+	union sockaddr_inany sa;
 	union flow *flow;
 	socklen_t sl;
 	int s, mss;
@@ -1956,12 +1947,12 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 
 	if (!c->no_map_gw) {
 		if (af == AF_INET && IN4_ARE_ADDR_EQUAL(daddr, &c->ip4.gw))
-			addr4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+			host_daddr = &in4addr_loopback;
 		if (af == AF_INET6 && IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.gw))
-			addr6.sin6_addr	= in6addr_loopback;
+			host_daddr = &in6addr_loopback;
 	}
 
-	if (af == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(&addr6.sin6_addr)) {
+	if (af == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(host_daddr)) {
 		struct sockaddr_in6 addr6_ll = {
 			.sin6_family = AF_INET6,
 			.sin6_addr = c->ip6.addr_ll,
@@ -1994,13 +1985,7 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 
 	inany_from_af(&conn->faddr, af, daddr);
 
-	if (af == AF_INET) {
-		sa = (struct sockaddr *)&addr4;
-		sl = sizeof(addr4);
-	} else {
-		sa = (struct sockaddr *)&addr6;
-		sl = sizeof(addr6);
-	}
+	sockaddr_inany_init(&sa, &sl, af, host_daddr, ntohs(th->dest), 0);
 
 	conn->fport = ntohs(th->dest);
 	conn->eport = ntohs(th->source);
@@ -2014,19 +1999,19 @@ static void tcp_conn_from_tap(struct ctx *c, sa_family_t af,
 
 	tcp_hash_insert(c, conn);
 
-	if (!bind(s, sa, sl)) {
+	if (!bind(s, &sa.sa, sl)) {
 		tcp_rst(c, conn);	/* Nobody is listening then */
 		return;
 	}
 	if (errno != EADDRNOTAVAIL && errno != EACCES)
 		conn_flag(c, conn, LOCAL);
 
-	if ((af == AF_INET &&  !IN4_IS_ADDR_LOOPBACK(&addr4.sin_addr)) ||
-	    (af == AF_INET6 && !IN6_IS_ADDR_LOOPBACK(&addr6.sin6_addr) &&
-			       !IN6_IS_ADDR_LINKLOCAL(&addr6.sin6_addr)))
+	if ((af == AF_INET &&  !IN4_IS_ADDR_LOOPBACK(&sa.sa4.sin_addr)) ||
+	    (af == AF_INET6 && !IN6_IS_ADDR_LOOPBACK(&sa.sa6.sin6_addr) &&
+			       !IN6_IS_ADDR_LINKLOCAL(&sa.sa6.sin6_addr)))
 		tcp_bind_outbound(c, s, af);
 
-	if (connect(s, sa, sl)) {
+	if (connect(s, &sa.sa, sl)) {
 		if (errno != EINPROGRESS) {
 			tcp_rst(c, conn);
 			return;
diff --git a/tcp_splice.c b/tcp_splice.c
index 3a2c0781..20f56ac3 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -327,17 +327,7 @@ static int tcp_splice_connect_finish(const struct ctx *c,
 static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn,
 			      int sock_conn, in_port_t port)
 {
-	struct sockaddr_in6 addr6 = {
-		.sin6_family = AF_INET6,
-		.sin6_port = htons(port),
-		.sin6_addr = IN6ADDR_LOOPBACK_INIT,
-	};
-	struct sockaddr_in addr4 = {
-		.sin_family = AF_INET,
-		.sin_port = htons(port),
-		.sin_addr = IN4ADDR_LOOPBACK_INIT,
-	};
-	const struct sockaddr *sa;
+	union sockaddr_inany sa;
 	socklen_t sl;
 
 	conn->s[1] = sock_conn;
@@ -348,15 +338,14 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn,
 			   conn->s[1]);
 	}
 
-	if (CONN_V6(conn)) {
-		sa = (struct sockaddr *)&addr6;
-		sl = sizeof(addr6);
-	} else {
-		sa = (struct sockaddr *)&addr4;
-		sl = sizeof(addr4);
-	}
+	if (CONN_V6(conn))
+		sockaddr_inany_init(&sa, &sl,
+				    AF_INET6, &in6addr_loopback, port, 0);
+	else
+		sockaddr_inany_init(&sa, &sl,
+				    AF_INET, &in4addr_loopback, port, 0);
 
-	if (connect(conn->s[1], sa, sl)) {
+	if (connect(conn->s[1], &sa.sa, sl)) {
 		if (errno != EINPROGRESS)
 			return -errno;
 		conn_event(c, conn, SPLICE_CONNECT);
-- 
@@ -327,17 +327,7 @@ static int tcp_splice_connect_finish(const struct ctx *c,
 static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn,
 			      int sock_conn, in_port_t port)
 {
-	struct sockaddr_in6 addr6 = {
-		.sin6_family = AF_INET6,
-		.sin6_port = htons(port),
-		.sin6_addr = IN6ADDR_LOOPBACK_INIT,
-	};
-	struct sockaddr_in addr4 = {
-		.sin_family = AF_INET,
-		.sin_port = htons(port),
-		.sin_addr = IN4ADDR_LOOPBACK_INIT,
-	};
-	const struct sockaddr *sa;
+	union sockaddr_inany sa;
 	socklen_t sl;
 
 	conn->s[1] = sock_conn;
@@ -348,15 +338,14 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn,
 			   conn->s[1]);
 	}
 
-	if (CONN_V6(conn)) {
-		sa = (struct sockaddr *)&addr6;
-		sl = sizeof(addr6);
-	} else {
-		sa = (struct sockaddr *)&addr4;
-		sl = sizeof(addr4);
-	}
+	if (CONN_V6(conn))
+		sockaddr_inany_init(&sa, &sl,
+				    AF_INET6, &in6addr_loopback, port, 0);
+	else
+		sockaddr_inany_init(&sa, &sl,
+				    AF_INET, &in4addr_loopback, port, 0);
 
-	if (connect(conn->s[1], sa, sl)) {
+	if (connect(conn->s[1], &sa.sa, sl)) {
 		if (errno != EINPROGRESS)
 			return -errno;
 		conn_event(c, conn, SPLICE_CONNECT);
-- 
2.43.0