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