From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202504 header.b=K5cHrXKV; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 517185A0275 for ; Thu, 17 Apr 2025 03:56:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1744854945; bh=BG3lwzYipqkKG4788+LnK1I7Zo/D+H3ldwE4hMnRs+I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=K5cHrXKVuCIVqUYaYTHXBI6bbmaR2eu2z2Cupis4pSJ9E/C+RwTp7J2lLroODL0f5 YAbu27vJf/Pm3NlWpBITbLjvou12a76YQ7tf7UjrS2y8N2NbUMZIKw97S9bc5d5pDe XnPokW0OYWJAvrmOoDHe6iSAWCPndXLY7oGLqID/u2qC1MKfY+9aiH1b6+NUVv6icG 8CJngXFC9w9bvanQduv6fptgKN20z1iGW4dXqL+pYfmDdnuZBfoIOFUeZIDhdssF1E 4tvGhfhLh6aZiLNeBY4RR88oRTYno7uT7uL/YVskI+sI7su4FQXH6wlYWvRwhR2zj9 YLt2FifYLkM2Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZdLZn6W4Tz4xS7; Thu, 17 Apr 2025 11:55:45 +1000 (AEST) From: David Gibson To: Stefano Brivio , passt-dev@passt.top, Jon Maloy Subject: [PATCH v2 2/4] treewide: Improve robustness against sockaddrs of unexpected family Date: Thu, 17 Apr 2025 11:55:41 +1000 Message-ID: <20250417015543.457310-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250417015543.457310-1-david@gibson.dropbear.id.au> References: <20250417015543.457310-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 5ISJFNMEBB7J7UPHCTY6B7ALKK3JKV6R X-Message-ID-Hash: 5ISJFNMEBB7J7UPHCTY6B7ALKK3JKV6R X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: inany_from_sockaddr() expects a socket address of family AF_INET or AF_INET6 and ASSERT()s if it gets anything else. In many of the callers we can handle an unexpected family more gracefully, though, e.g. by failing a single flow rather than killing passt. Change inany_from_sockaddr() to return an error instead of ASSERT()ing, and handle those errors in the callers. Improve the reporting of any such errors while we're at it. With this greater robustness, allow inany_from_sockaddr() to take a void * rather than specifically a union sockaddr_inany *. Signed-off-by: David Gibson --- flow.c | 16 ++++++++++++++-- inany.h | 28 +++++++++++++++++----------- tcp.c | 10 ++++------ udp_flow.c | 6 +++--- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/flow.c b/flow.c index 3c81cb42..447c0211 100644 --- a/flow.c +++ b/flow.c @@ -408,7 +408,12 @@ struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif, { struct flowside *ini = &flow->f.side[INISIDE]; - inany_from_sockaddr(&ini->eaddr, &ini->eport, ssa); + if (inany_from_sockaddr(&ini->eaddr, &ini->eport, ssa) < 0) { + char str[SOCKADDR_STRLEN]; + + ASSERT_WITH_MSG(0, "Bad socket address %s", + sockaddr_ntop(ssa, str, sizeof(str))); + } if (daddr) ini->oaddr = *daddr; else if (inany_v4(&ini->eaddr)) @@ -768,7 +773,14 @@ flow_sidx_t flow_lookup_sa(const struct ctx *c, uint8_t proto, uint8_t pif, .oport = oport, }; - inany_from_sockaddr(&side.eaddr, &side.eport, esa); + if (inany_from_sockaddr(&side.eaddr, &side.eport, esa) < 0) { + char str[SOCKADDR_STRLEN]; + + warn("Flow lookup on bad socket address %s", + sockaddr_ntop(esa, str, sizeof(str))); + return FLOW_SIDX_NONE; + } + if (oaddr) side.oaddr = *oaddr; else if (inany_v4(&side.eaddr)) diff --git a/inany.h b/inany.h index 1c247e1e..7ca5cbd3 100644 --- a/inany.h +++ b/inany.h @@ -237,24 +237,30 @@ static inline void inany_from_af(union inany_addr *aa, } /** inany_from_sockaddr - Extract IPv[46] address and port number from sockaddr - * @aa: Pointer to store IPv[46] address + * @dst: Pointer to store IPv[46] address (output) * @port: Pointer to store port number, host order - * @addr: AF_INET or AF_INET6 socket address + * @addr: Socket address + * + * Return: 0 on success, -1 on error (bad address family) */ -static inline void inany_from_sockaddr(union inany_addr *aa, in_port_t *port, - const union sockaddr_inany *sa) +static inline int inany_from_sockaddr(union inany_addr *dst, in_port_t *port, + const void *addr) { + const union sockaddr_inany *sa = (const union sockaddr_inany *)addr; + if (sa->sa_family == AF_INET6) { - inany_from_af(aa, AF_INET6, &sa->sa6.sin6_addr); + inany_from_af(dst, AF_INET6, &sa->sa6.sin6_addr); *port = ntohs(sa->sa6.sin6_port); - } else if (sa->sa_family == AF_INET) { - inany_from_af(aa, AF_INET, &sa->sa4.sin_addr); + return 0; + } + + if (sa->sa_family == AF_INET) { + inany_from_af(dst, AF_INET, &sa->sa4.sin_addr); *port = ntohs(sa->sa4.sin_port); - } else { - /* Not valid to call with other address families */ - ASSERT_WITH_MSG(0, "Unexpected sockaddr family: %u", - sa->sa_family); + return 0; } + + return -1; } /** inany_siphash_feed- Fold IPv[46] address into an in-progress siphash diff --git a/tcp.c b/tcp.c index 9c6bc529..0ac298a7 100644 --- a/tcp.c +++ b/tcp.c @@ -1546,9 +1546,8 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, if (c->mode == MODE_VU) { /* To rebind to same oport after migration */ sl = sizeof(sa); - if (!getsockname(s, &sa.sa, &sl)) - inany_from_sockaddr(&tgt->oaddr, &tgt->oport, &sa); - else + 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); } @@ -2204,9 +2203,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref, NULL, ref.tcp_listen.port); if (c->mode == MODE_VU) { /* Rebind to same address after migration */ - if (!getsockname(s, &sa.sa, &sl)) - inany_from_sockaddr(&ini->oaddr, &ini->oport, &sa); - else + if (getsockname(s, &sa.sa, &sl) || + inany_from_sockaddr(&ini->oaddr, &ini->oport, &sa) < 0) err_perror("Can't get local address for socket %i", s); } diff --git a/udp_flow.c b/udp_flow.c index ef2cbb06..fea1cf3c 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -158,12 +158,12 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, socklen_t sl = sizeof(sa); in_port_t port; - if (getsockname(uflow->s[TGTSIDE], &sa.sa, &sl) < 0) { + if (getsockname(uflow->s[TGTSIDE], &sa.sa, &sl) < 0 || + inany_from_sockaddr(&uflow->f.side[TGTSIDE].oaddr, + &port, &sa) < 0) { flow_perror(uflow, "Unable to determine local address"); goto cancel; } - inany_from_sockaddr(&uflow->f.side[TGTSIDE].oaddr, - &port, &sa); if (port != tgt->oport) { flow_err(uflow, "Unexpected local port"); goto cancel; -- 2.49.0