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=HybI3VZM; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id B27A85A0272 for ; Wed, 09 Apr 2025 08:35:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202504; t=1744180543; bh=/3y5LkuLgpGY/AoQwTQzblnWVKLp0ps6A7+Hgzg7N3U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HybI3VZMJrrKzB/bFCuO8lb66CLPb7pgP9+e+bD12VyIt81jtCBVDORnGFSj94q85 xwqV4GFUXxIvQkx/WkQQbM1aYF+kJIdXQK5rw3P4DgKGlMibC1Ixdik2Ek7G2j4e9z rKW5lwte1BFHdHj++lm8HQX/ELMcNl4DTy/+ss/HJg0KFVUWG/GNRRFgiJ9CjO0CAf cWNUXpgersgZFuq7xJAe3Ci1b7SYUpojHw2wm0mckh7GqJXJVdgxG1cT0XynyEYQgv Xm9G93sI9orNfJ+CxqPG6GRQwagrlrEB/v4d+ZYc1LihYrDRcmqeEB69Uzhn+IZQ7u V0Vb725WLHX1A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZXY9W48KJz4wcQ; Wed, 9 Apr 2025 16:35:43 +1000 (AEST) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 1/2] tcp_splice: Don't double could bytes read on EINTR Date: Wed, 9 Apr 2025 16:35:40 +1000 Message-ID: <20250409063541.1411177-2-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250409063541.1411177-1-david@gibson.dropbear.id.au> References: <20250409063541.1411177-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: OZJLS6B2C2J4GQ3DWPUPF35E3ZQMETOK X-Message-ID-Hash: OZJLS6B2C2J4GQ3DWPUPF35E3ZQMETOK 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: In tcp_splice_sock_handler(), if we get an EINTR on our second splice() (pipe to output socket) we - as we should - go back and retry it. However, we do so *after* we've already updated our byte counters. That does no harm for the conn->written[] counter - since the second splice() returned an error it will be advanced by 0. However we also advance the conn->read[] counter, and then do so again when the splice() succeeds. This results in the counters being out of sync, and us thinking we have remaining data in the pipe when we don't, which can leave us in an infinite loop once the stream finishes. Fix this by moving the EINTR handling to directly next to the splice() call (which is what we usually do for EINTR). As a bonus this removes one mildly confusing goto. For symmetry, also rework the EINTR handling on the first splice() the same way, although that doesn't (as far as I can tell) have buggy side effects. Link: https://github.com/containers/podman/issues/23686 Signed-off-by: David Gibson --- tcp_splice.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index 0d10e3d4..7c3b56f9 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -520,15 +520,14 @@ swap: int more = 0; retry: - readlen = splice(conn->s[fromsidei], NULL, - conn->pipe[fromsidei][1], NULL, - c->tcp.pipe_size, - SPLICE_F_MOVE | SPLICE_F_NONBLOCK); + do + readlen = splice(conn->s[fromsidei], NULL, + conn->pipe[fromsidei][1], NULL, + c->tcp.pipe_size, + SPLICE_F_MOVE | SPLICE_F_NONBLOCK); + while (readlen < 0 && errno == EINTR); flow_trace(conn, "%zi from read-side call", readlen); if (readlen < 0) { - if (errno == EINTR) - goto retry; - if (errno != EAGAIN) goto close; } else if (!readlen) { @@ -543,10 +542,13 @@ retry: conn_flag(c, conn, lowat_act_flag); } -eintr: - written = splice(conn->pipe[fromsidei][0], NULL, - conn->s[!fromsidei], NULL, c->tcp.pipe_size, - SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); + do + written = splice(conn->pipe[fromsidei][0], NULL, + conn->s[!fromsidei], NULL, + c->tcp.pipe_size, + SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); + while (written < 0 && errno == EINTR); + flow_trace(conn, "%zi from write-side call (passed %zi)", written, c->tcp.pipe_size); @@ -578,9 +580,6 @@ eintr: conn->written[fromsidei] += written > 0 ? written : 0; if (written < 0) { - if (errno == EINTR) - goto eintr; - if (errno != EAGAIN) goto close; -- 2.49.0