sockaddr_inany can contain either an IPv4 or IPv6 socket address, so the
relevant length for bind() or connect() can vary. In pif_sockaddr() we
return that length, and in sock_l4_sa() we take it as an extra parameter.
However, sockaddr_inany always contains exactly a sockaddr_in or a
sockaddr_in6 each with a fixed size. Therefore we can derive the relevant
length from the family, and don't need to pass it around separately.
Make a tiny helper to get the relevant address length, and update all
interfaces to use that approach instead.
In the process, fix a buglet in tcp_flow_repair_bind(): we passed
sizeof(union sockaddr_inany) to bind() instead of the specific length for
the address family. Since the sizeof() is always longer than the specific
length, this is probably fine, but not theoretically correct.
Signed-off-by: David Gibson
---
flow.c | 17 +++++++----------
icmp.c | 3 ++-
inany.h | 18 ++++++++++++++++++
pif.c | 14 ++++----------
pif.h | 2 +-
tcp.c | 20 +++++++++-----------
tcp_splice.c | 5 ++---
udp.c | 8 +++-----
util.c | 9 ++++-----
util.h | 5 +++--
10 files changed, 53 insertions(+), 48 deletions(-)
diff --git a/flow.c b/flow.c
index feefda3c..9926f408 100644
--- a/flow.c
+++ b/flow.c
@@ -170,8 +170,7 @@ struct flowside_sock_args {
int fd;
int err;
enum epoll_type type;
- const struct sockaddr *sa;
- socklen_t sl;
+ const union sockaddr_inany *sa;
const char *path;
uint32_t data;
};
@@ -187,7 +186,7 @@ static int flowside_sock_splice(void *arg)
ns_enter(a->c);
- a->fd = sock_l4_sa(a->c, a->type, a->sa, a->sl, NULL,
+ a->fd = sock_l4_sa(a->c, a->type, a->sa, NULL,
a->sa->sa_family == AF_INET6, a->data);
a->err = errno;
@@ -209,11 +208,10 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
{
const char *ifname = NULL;
union sockaddr_inany sa;
- socklen_t sl;
ASSERT(pif_is_socket(pif));
- pif_sockaddr(c, &sa, &sl, pif, &tgt->oaddr, tgt->oport);
+ pif_sockaddr(c, &sa, pif, &tgt->oaddr, tgt->oport);
switch (pif) {
case PIF_HOST:
@@ -224,13 +222,13 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
else if (sa.sa_family == AF_INET6)
ifname = c->ip6.ifname_out;
- return sock_l4_sa(c, type, &sa, sl, ifname,
+ return sock_l4_sa(c, type, &sa, ifname,
sa.sa_family == AF_INET6, data);
case PIF_SPLICE: {
struct flowside_sock_args args = {
.c = c, .type = type,
- .sa = &sa.sa, .sl = sl, .data = data,
+ .sa = &sa, .data = data,
};
NS_CALL(flowside_sock_splice, &args);
errno = args.err;
@@ -259,10 +257,9 @@ int flowside_connect(const struct ctx *c, int s,
uint8_t pif, const struct flowside *tgt)
{
union sockaddr_inany sa;
- socklen_t sl;
- pif_sockaddr(c, &sa, &sl, pif, &tgt->eaddr, tgt->eport);
- return connect(s, &sa.sa, sl);
+ pif_sockaddr(c, &sa, pif, &tgt->eaddr, tgt->eport);
+ return connect(s, &sa.sa, socklen_inany(&sa));
}
/** flow_log_ - Log flow-related message
diff --git a/icmp.c b/icmp.c
index 6dffafb0..62b3bed8 100644
--- a/icmp.c
+++ b/icmp.c
@@ -301,8 +301,9 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
ASSERT(flow_proto[pingf->f.type] == proto);
pingf->ts = now->tv_sec;
- pif_sockaddr(c, &sa, &msh.msg_namelen, PIF_HOST, &tgt->eaddr, 0);
+ pif_sockaddr(c, &sa, PIF_HOST, &tgt->eaddr, 0);
msh.msg_name = &sa;
+ msh.msg_namelen = socklen_inany(&sa);
msh.msg_iov = iov;
msh.msg_iovlen = cnt;
msh.msg_control = NULL;
diff --git a/inany.h b/inany.h
index 7ca5cbd3..e3cae2a8 100644
--- a/inany.h
+++ b/inany.h
@@ -67,6 +67,24 @@ union sockaddr_inany {
struct sockaddr_in6 sa6;
};
+/** socklen_inany() - Return relevant size of an sockaddr_inany
+ * @sa: sockaddr_iany socket address
+ *
+ * Returns the correct socket address length to pass to bind() or connect() with
+ * @sa, based on whether it is an IPv4 or IPv6 address.
+ */
+static inline socklen_t socklen_inany(const union sockaddr_inany *sa)
+{
+ switch (sa->sa_family) {
+ case AF_INET:
+ return sizeof(sa->sa4);
+ case AF_INET6:
+ return sizeof(sa->sa6);
+ default:
+ ASSERT(0);
+ }
+}
+
/** inany_v4 - Extract IPv4 address, if present, from IPv[46] address
* @addr: IPv4 or IPv6 address
*
diff --git a/pif.c b/pif.c
index 592fafaa..31723b29 100644
--- a/pif.c
+++ b/pif.c
@@ -29,12 +29,11 @@ static_assert(ARRAY_SIZE(pif_type_str) == PIF_NUM_TYPES,
/** pif_sockaddr() - Construct a socket address suitable for an interface
* @c: Execution context
* @sa: Pointer to sockaddr to fill in
- * @sl: Updated to relevant length of initialised @sa
* @pif: Interface to create the socket address
* @addr: IPv[46] address
* @port: Port (host byte order)
*/
-void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
+void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa,
uint8_t pif, const union inany_addr *addr, in_port_t port)
{
const struct in_addr *v4 = inany_v4(addr);
@@ -46,7 +45,6 @@ void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
sa->sa4.sin_addr = *v4;
sa->sa4.sin_port = htons(port);
memset(&sa->sa4.sin_zero, 0, sizeof(sa->sa4.sin_zero));
- *sl = sizeof(sa->sa4);
} else {
sa->sa_family = AF_INET6;
sa->sa6.sin6_addr = addr->a6;
@@ -56,7 +54,6 @@ void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
else
sa->sa6.sin6_scope_id = 0;
sa->sa6.sin6_flowinfo = 0;
- *sl = sizeof(sa->sa6);
}
}
@@ -83,7 +80,6 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
.sa6.sin6_addr = in6addr_any,
.sa6.sin6_port = htons(port),
};
- socklen_t sl;
ASSERT(pif_is_socket(pif));
@@ -94,10 +90,8 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
}
if (!addr)
- return sock_l4_sa(c, type, &sa, sizeof(sa.sa6),
- ifname, false, data);
+ return sock_l4_sa(c, type, &sa, ifname, false, data);
- pif_sockaddr(c, &sa, &sl, pif, addr, port);
- return sock_l4_sa(c, type, &sa, sl,
- ifname, sa.sa_family == AF_INET6, data);
+ pif_sockaddr(c, &sa, pif, addr, port);
+ return sock_l4_sa(c, type, &sa, ifname, sa.sa_family == AF_INET6, data);
}
diff --git a/pif.h b/pif.h
index f029282b..0f7f6672 100644
--- a/pif.h
+++ b/pif.h
@@ -57,7 +57,7 @@ static inline bool pif_is_socket(uint8_t pif)
return pif == PIF_HOST || pif == PIF_SPLICE;
}
-void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl,
+void pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa,
uint8_t pif, const union inany_addr *addr, in_port_t port);
int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif,
const union inany_addr *addr, const char *ifname,
diff --git a/tcp.c b/tcp.c
index 0f9e9b3f..d63a16bf 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1443,12 +1443,11 @@ static void tcp_bind_outbound(const struct ctx *c,
{
const struct flowside *tgt = &conn->f.side[TGTSIDE];
union sockaddr_inany bind_sa;
- socklen_t sl;
- pif_sockaddr(c, &bind_sa, &sl, PIF_HOST, &tgt->oaddr, tgt->oport);
+ pif_sockaddr(c, &bind_sa, PIF_HOST, &tgt->oaddr, tgt->oport);
if (!inany_is_unspecified(&tgt->oaddr) || tgt->oport) {
- if (bind(s, &bind_sa.sa, sl)) {
+ if (bind(s, &bind_sa.sa, socklen_inany(&bind_sa))) {
char sstr[INANY_ADDRSTRLEN];
flow_dbg_perror(conn,
@@ -1508,7 +1507,6 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
union flow *flow;
int s = -1, mss;
uint64_t hash;
- socklen_t sl;
if (!(flow = flow_alloc()))
return;
@@ -1541,7 +1539,7 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
if ((s = tcp_conn_sock(af)) < 0)
goto cancel;
- pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, tgt->eport);
+ pif_sockaddr(c, &sa, PIF_HOST, &tgt->eaddr, tgt->eport);
/* Use bind() to check if the target address is local (EADDRINUSE or
* similar) and already bound, and set the LOCAL flag in that case.
@@ -1553,7 +1551,7 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
*
* So, if bind() succeeds, close the socket, get a new one, and proceed.
*/
- if (bind(s, &sa.sa, sl)) {
+ if (bind(s, &sa.sa, socklen_inany(&sa))) {
if (errno != EADDRNOTAVAIL && errno != EACCES)
conn_flag(c, conn, LOCAL);
} else {
@@ -1593,7 +1591,7 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
tcp_bind_outbound(c, conn, s);
- if (connect(s, &sa.sa, sl)) {
+ if (connect(s, &sa.sa, socklen_inany(&sa))) {
if (errno != EINPROGRESS) {
tcp_rst(c, conn);
goto cancel;
@@ -1612,7 +1610,8 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
tcp_epoll_ctl(c, conn);
if (c->mode == MODE_VU) { /* To rebind to same oport after migration */
- sl = sizeof(sa);
+ socklen_t sl = sizeof(sa);
+
if (getsockname(s, &sa.sa, &sl) ||
inany_from_sockaddr(&tgt->oaddr, &tgt->oport, &sa) < 0)
err_perror("Can't get local address for socket %i", s);
@@ -3592,11 +3591,10 @@ static int tcp_flow_repair_bind(const struct ctx *c, struct tcp_tap_conn *conn)
{
const struct flowside *sockside = HOSTFLOW(conn);
union sockaddr_inany a;
- socklen_t sl;
- pif_sockaddr(c, &a, &sl, PIF_HOST, &sockside->oaddr, sockside->oport);
+ pif_sockaddr(c, &a, PIF_HOST, &sockside->oaddr, sockside->oport);
- if (bind(conn->sock, &a.sa, sizeof(a))) {
+ if (bind(conn->sock, &a.sa, socklen_inany(&a))) {
int rc = -errno;
flow_perror(conn, "Failed to bind socket for migrated flow");
return rc;
diff --git a/tcp_splice.c b/tcp_splice.c
index 26cb6306..174a64d9 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -351,7 +351,6 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn)
sa_family_t af = inany_v4(&tgt->eaddr) ? AF_INET : AF_INET6;
uint8_t tgtpif = conn->f.pif[TGTSIDE];
union sockaddr_inany sa;
- socklen_t sl;
int one = 1;
if (tgtpif == PIF_HOST)
@@ -379,9 +378,9 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn)
conn->s[1]);
}
- pif_sockaddr(c, &sa, &sl, tgtpif, &tgt->eaddr, tgt->eport);
+ pif_sockaddr(c, &sa, tgtpif, &tgt->eaddr, tgt->eport);
- if (connect(conn->s[1], &sa.sa, sl)) {
+ if (connect(conn->s[1], &sa.sa, socklen_inany(&sa))) {
if (errno != EINPROGRESS) {
flow_trace(conn, "Couldn't connect socket for splice: %s",
strerror_(errno));
diff --git a/udp.c b/udp.c
index 86585b7e..8f96495d 100644
--- a/udp.c
+++ b/udp.c
@@ -773,7 +773,6 @@ static void udp_sock_to_sock(const struct ctx *c, int from_s, int n,
const struct udp_flow *uflow = udp_at_sidx(tosidx);
uint8_t topif = pif_at_sidx(tosidx);
int to_s = uflow->s[tosidx.sidei];
- socklen_t sl;
int i;
if ((n = udp_sock_recv(c, from_s, udp_mh_recv, n)) <= 0)
@@ -784,7 +783,7 @@ static void udp_sock_to_sock(const struct ctx *c, int from_s, int n,
= udp_mh_recv[i].msg_len;
}
- pif_sockaddr(c, &udp_splice_to, &sl, topif,
+ pif_sockaddr(c, &udp_splice_to, topif,
&toside->eaddr, toside->eport);
sendmmsg(to_s, udp_mh_splice, n, MSG_NOSIGNAL);
@@ -986,7 +985,6 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
flow_sidx_t tosidx;
in_port_t src, dst;
uint8_t topif;
- socklen_t sl;
ASSERT(!c->no_udp);
@@ -1028,7 +1026,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
s = uflow->s[tosidx.sidei];
ASSERT(s >= 0);
- pif_sockaddr(c, &to_sa, &sl, topif, &toside->eaddr, toside->eport);
+ pif_sockaddr(c, &to_sa, topif, &toside->eaddr, toside->eport);
for (i = 0, j = 0; i < (int)p->count - idx && j < UIO_MAXIOV; i++) {
const struct udphdr *uh_send;
@@ -1041,7 +1039,7 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif,
return p->count - idx;
mm[i].msg_hdr.msg_name = &to_sa;
- mm[i].msg_hdr.msg_namelen = sl;
+ mm[i].msg_hdr.msg_namelen = socklen_inany(&to_sa);
if (data.cnt) {
int cnt;
diff --git a/util.c b/util.c
index c492f904..976fcabe 100644
--- a/util.c
+++ b/util.c
@@ -44,7 +44,6 @@
* @c: Execution context
* @type: epoll type
* @sa: Socket address to bind to
- * @sl: Length of @sa
* @ifname: Interface for binding, NULL for any
* @v6only: Set IPV6_V6ONLY socket option
* @data: epoll reference portion for protocol handlers
@@ -52,10 +51,10 @@
* Return: newly created socket, negative error code on failure
*/
int sock_l4_sa(const struct ctx *c, enum epoll_type type,
- const void *sa, socklen_t sl,
- const char *ifname, bool v6only, uint32_t data)
+ const union sockaddr_inany *sa, const char *ifname,
+ bool v6only, uint32_t data)
{
- sa_family_t af = ((const struct sockaddr *)sa)->sa_family;
+ sa_family_t af = sa->sa_family;
union epoll_ref ref = { .type = type, .data = data };
bool freebind = false;
struct epoll_event ev;
@@ -152,7 +151,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,
}
}
- if (bind(fd, sa, sl) < 0) {
+ if (bind(fd, &sa->sa, socklen_inany(sa)) < 0) {
/* We'll fail to bind to low ports if we don't have enough
* capabilities, and we'll fail to bind on already bound ports,
* this is fine. This might also fail for ICMP because of a
diff --git a/util.h b/util.h
index 22eaac56..e1a1ebc9 100644
--- a/util.h
+++ b/util.h
@@ -201,10 +201,11 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
#include "packet.h"
struct ctx;
+union sockaddr_inany;
int sock_l4_sa(const struct ctx *c, enum epoll_type type,
- const void *sa, socklen_t sl,
- const char *ifname, bool v6only, uint32_t data);
+ const union sockaddr_inany *sa, const char *ifname,
+ bool v6only, uint32_t data);
int sock_unix(char *sock_path);
void sock_probe_mem(struct ctx *c);
long timespec_diff_ms(const struct timespec *a, const struct timespec *b);
--
2.51.0