From d3f33f3b8ec4646dae3584b648cba142a73d3208 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 9 Apr 2025 16:35:40 +1000 Subject: tcp_splice: Don't double count bytes read on EINTR 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#issuecomment-2779347687 Signed-off-by: David Gibson Signed-off-by: Stefano Brivio --- tcp_splice.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tcp_splice.c b/tcp_splice.c index 0d10e3d..7c3b56f 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; -- cgit v1.2.3