From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 64BC55A026D for ; Mon, 6 Nov 2023 03:17:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1699237034; bh=pzTeniA6DtXOkgJJO6rZpd6Zt2rJ4slGpwG9LTo1zjY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=E756yhrIRjjBZjSQYe2SqTTPU065IJVFBy0i4pE42oj/9sZdmgzYm0UpPGyyITiMw IokF/bCTbTYKd13+Vryr34IjaSUwiL4WuOr/A8ugOFy1fC7P7bOavgYTPfoQza1kyE HLxMFv38tHdXlzgDzTjczt3aqcCKoXvdL5wEgZ6I= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4SNw3G1RJgz4xPc; Mon, 6 Nov 2023 13:17:14 +1100 (AEDT) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 2/2] udp: Remove socket from udp_{tap,splice}_map when timed out Date: Mon, 6 Nov 2023 13:17:09 +1100 Message-ID: <20231106021709.603571-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231106021709.603571-1-david@gibson.dropbear.id.au> References: <20231106021709.603571-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 4OEHLNKM3PYKAUYAH7E5SUDQG5CUYNCP X-Message-ID-Hash: 4OEHLNKM3PYKAUYAH7E5SUDQG5CUYNCP 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: bugs.passt.top@bitsbetwixt.com, 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: We save sockets bound to particular ports in udp_{tap,splice}_map for reuse later. If they're not used for a time, we time them out and close them. However, when that happened, we weren't actually removing the fds from the relevant map. That meant that later interactions on the same port could get a stale fd from the map. The stale fd might be closed, leading to unexpected EBADF errors, or it could have been re-used by a completely different socket bound to a different port, which could lead to us incorrectly forwarding packets. Link: https://bugs.passt.top/show_bug.cgi?id=57 Signed-off-by: David Gibson --- udp.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/udp.c b/udp.c index a8473e3..b40d1f3 100644 --- a/udp.c +++ b/udp.c @@ -1146,14 +1146,14 @@ static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type type, { struct udp_splice_port *sp; struct udp_tap_port *tp; - int s = -1; + int *sockp = NULL; switch (type) { case UDP_ACT_TAP: tp = &udp_tap_map[v6 ? V6 : V4][port]; if (ts->tv_sec - tp->ts > UDP_CONN_TIMEOUT) { - s = tp->sock; + sockp = &tp->sock; tp->flags = 0; } @@ -1162,21 +1162,23 @@ static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type type, sp = &udp_splice_init[v6 ? V6 : V4][port]; if (ts->tv_sec - sp->ts > UDP_CONN_TIMEOUT) - s = sp->sock; + sockp = &sp->sock; break; case UDP_ACT_SPLICE_NS: sp = &udp_splice_ns[v6 ? V6 : V4][port]; if (ts->tv_sec - sp->ts > UDP_CONN_TIMEOUT) - s = sp->sock; + sockp = &sp->sock; break; default: return; } - if (s >= 0) { + if (sockp && *sockp >= 0) { + int s = *sockp; + *sockp = -1; epoll_ctl(c->epollfd, EPOLL_CTL_DEL, s, NULL); close(s); bitmap_clear(udp_act[v6 ? V6 : V4][type], port); -- 2.41.0