[PATCH bpf 0/2] bpf, sockmap: Fix dropped data and EPIPE on empty skb redirect

[PATCH bpf 0/2] bpf, sockmap: Fix dropped data and EPIPE on empty skb redirect

[Eli Skeggs via B4 Relay]

When a socket receives a bare FIN, tcp_read_skb() hands an empty skb to
the attached BPF stream_verdict program. The program may redirect that
skb to another socket. Doing so causes the target socket to report EPIPE
and stop transmitting redirected data. A proxy may see the erroneous
EPIPE and truncate a half-closed connection. Instead of treating a
redirected empty skb as an error, treat a 0 return as an error only when
there is more data to send.

Patch 2 adds a selftest that fails without patch 1.

Tested on x86_64 with virtme-ng, under QEMU without KVM. The new subtest
fails on bpf/master without patch 1, and passes with it. The full
test_progs -t sockmap passes all seven tests and 802 subtests with patch
1.

The bug dates from v6.0 (965b57b469a5), so please consider queueing
patch 1 for stable.

Found while building a sockmap proxy between TCP and MPTCP sockets,
running out-of-tree MPTCP sockmap patches, where the EPIPE caused
half-closed connections to prematurely abort. An LLM helped locate and
reproduce the issue, and proposed an initial fix. I traced the code
path, reviewed and verified the LLM-written test, and simplified its
proposal to a one-line fix.

Signed-off-by: Eli Skeggs <kernel@eliskeggs.com>
---
Eli Skeggs (2):
      bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect
      selftests/bpf: Redirect a bare FIN to another socket's egress

 net/core/skmsg.c                                   |  2 +-
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 74 ++++++++++++++++++++++
 .../selftests/bpf/progs/test_sockmap_pass_prog.c   |  8 +++
 3 files changed, 83 insertions(+), 1 deletion(-)
---
base-commit: f909e534f5e34feb2ea2701d54df63e21aa506fc
change-id: 20261020-sockmap-bare-fin-e1f8160c3c7b

Best regards,
--  
Eli Skeggs <kernel@eliskeggs.com>

[PATCH bpf 1/2] bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect

[Eli Skeggs via B4 Relay]

From: Eli Skeggs <kernel@eliskeggs.com>

A FIN can arrive on a packet with no data. In that case, tcp_read_skb()
produces an empty skb, and the attached BPF stream_verdict program may
choose to redirect the skb to another socket. Further data redirected to
that socket from any socket gets dropped, and the target socket starts
reporting EPIPE. A userspace application may see that EPIPE and decide
the socket is no longer usable. For a BPF-based proxy, this can break a
bidirectional stream, truncating the response instead of leaving the
socket half-open.

Prior to the introduction of tcp_read_skb(), tcp_read_sock() never
passed a zero-length skb to its actor, sk_psock_verdict_recv(). When a
redirected skb landed in sk_psock_backlog(), it was always non-empty.
tcp_read_skb() deviated from the historical behavior by passing any
zero-length skbs to recv_actor(), exposing this latent bug by adding
zero-length skbs to a queue that hadn't previously received them.  When
sk_psock_backlog() encounters a zero-length skb, it interprets ret == 0
as a hard error, reports EPIPE, and clears the transmit flag.

Treat a return of 0 as success when there is no data to send, while
continuing to fail on a 0 return with residual bytes. The loop exits
without incident on an empty skb, and the skb gets cleaned up as usual.
On an ingress redirect, the empty message is still queued as before, but
the target socket no longer reports EPIPE.

Fixes: 965b57b469a5 ("net: Introduce a new proto_ops ->read_skb()")
Signed-off-by: Eli Skeggs <kernel@eliskeggs.com>
---
 net/core/skmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index df385a5a961e..f9b2e3486cfb 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -716,7 +716,7 @@ static void sk_psock_backlog(struct work_struct *work)
 			if (!sock_flag(psock->sk, SOCK_DEAD))
 				ret = sk_psock_handle_skb(psock, skb, off,
 							  len, ingress);
-			if (ret <= 0) {
+			if (ret < 0 || (!ret && len)) {
 				if (ret == -EAGAIN) {
 					sk_psock_skb_state(psock, state, len, off);
 					/* Restore redir info we cleared before */

-- 
2.56.0

[PATCH bpf 2/2] selftests/bpf: Redirect a bare FIN to another socket's egress

[Eli Skeggs via B4 Relay]

From: Eli Skeggs <kernel@eliskeggs.com>

Redirect data from two sockets to a third socket's egress. Half-close
the first socket so that its FIN arrives in an skb of its own, then
confirm that data redirected from the second socket after the first
socket's half-close still egresses from the third socket, and that
the third socket reports no error.

Without the sk_psock_backlog() fix, the zero-length skb breaks the
target's psock: it ends up with a disabled transmit flag, and SO_ERROR
reports EPIPE.

Signed-off-by: Eli Skeggs <kernel@eliskeggs.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 74 ++++++++++++++++++++++
 .../selftests/bpf/progs/test_sockmap_pass_prog.c   |  8 +++
 2 files changed, 82 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 1fef6ec2ba7a..e1e477ce72b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1325,6 +1325,78 @@ static void test_sockmap_multi_channels(int sotype)
 	test_sockmap_pass_prog__destroy(skel);
 }
 
+/* A bare FIN turns into an empty skb. When redirected to another socket,
+ * verify that it does not break the destination socket's psock. Data
+ * later redirected to the destination socket must still transmit.
+ */
+static void test_sockmap_skb_verdict_redir_fin(void)
+{
+	int c0 = -1, p0 = -1, c1 = -1, p1 = -1, c2 = -1, p2 = -1;
+	int err, map, zero = 0, one = 1, two = 2, sock_err;
+	struct test_sockmap_pass_prog *skel;
+	socklen_t len = sizeof(sock_err);
+	struct bpf_program *prog;
+	char buf = 'x', rcv;
+
+	skel = test_sockmap_pass_prog__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+	if (!ASSERT_OK(err, "create_socket_pairs()"))
+		goto end;
+	err = create_pair(AF_INET, SOCK_STREAM, &c2, &p2);
+	if (!ASSERT_OK(err, "create_pair()"))
+		goto end;
+
+	map = bpf_map__fd(skel->maps.sock_map_rx);
+	prog = skel->progs.prog_skb_verdict_egress;
+	err = bpf_prog_attach(bpf_program__fd(prog), map,
+			      BPF_SK_SKB_STREAM_VERDICT, 0);
+	if (!ASSERT_OK(err, "bpf_prog_attach verdict"))
+		goto end;
+
+	err = bpf_map_update_elem(map, &zero, &p0, BPF_ANY);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(p0)"))
+		goto end;
+	err = bpf_map_update_elem(map, &one, &p1, BPF_ANY);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(p1)"))
+		goto end;
+	err = bpf_map_update_elem(map, &two, &p2, BPF_ANY);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(p2)"))
+		goto end;
+
+	if (!ASSERT_EQ(xsend(c0, &buf, 1, 0), 1, "xsend(c0)"))
+		goto end;
+	if (!ASSERT_EQ(recv_timeout(c1, &rcv, 1, 0, IO_TIMEOUT_SEC), 1,
+		       "recv_timeout(c1)"))
+		goto end;
+
+	err = shutdown(c0, SHUT_WR);
+	if (!ASSERT_OK(err, "shutdown(c0)"))
+		goto end;
+	if (!ASSERT_OK(recv_timeout(p0, &rcv, 1, 0, IO_TIMEOUT_SEC),
+		       "recv_timeout(p0) EOF"))
+		goto end;
+
+	if (!ASSERT_EQ(xsend(c2, &buf, 1, 0), 1, "xsend(c2)"))
+		goto end;
+	ASSERT_EQ(recv_timeout(c1, &rcv, 1, 0, IO_TIMEOUT_SEC), 1,
+		  "recv_timeout(c1) after FIN");
+
+	err = getsockopt(p1, SOL_SOCKET, SO_ERROR, &sock_err, &len);
+	if (ASSERT_OK(err, "getsockopt(SO_ERROR)"))
+		ASSERT_OK(sock_err, "p1 error");
+end:
+	close_fd(&c0);
+	close_fd(&p0);
+	close_fd(&c1);
+	close_fd(&p1);
+	close_fd(&c2);
+	close_fd(&p2);
+	test_sockmap_pass_prog__destroy(skel);
+}
+
 /* A socket in a sockmap without a verdict program keeps its ingress data
  * in sk_receive_queue: FIONREAD must account for it.
  */
@@ -1404,6 +1476,8 @@ void test_sockmap_basic(void)
 		test_sockmap_no_verdict_fionread();
 	if (test__start_subtest("sockmap skb_verdict fionread on drop"))
 		test_sockmap_skb_verdict_fionread(false);
+	if (test__start_subtest("sockmap skb_verdict redirect bare FIN"))
+		test_sockmap_skb_verdict_redir_fin();
 	if (test__start_subtest("sockmap skb_verdict change tail"))
 		test_sockmap_skb_verdict_change_tail();
 	if (test__start_subtest("sockmap msg_verdict pop_data overflow"))
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c b/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
index ef9edca184ea..dd3cb7524e21 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
@@ -52,6 +52,14 @@ int prog_skb_verdict_ingress(struct __sk_buff *skb)
 	return bpf_sk_redirect_map(skb, &sock_map_rx, one, BPF_F_INGRESS);
 }
 
+SEC("sk_skb/stream_verdict")
+int prog_skb_verdict_egress(struct __sk_buff *skb)
+{
+	int one = 1;
+
+	return bpf_sk_redirect_map(skb, &sock_map_rx, one, 0);
+}
+
 SEC("sk_skb/stream_parser")
 int prog_skb_verdict_ingress_strp(struct __sk_buff *skb)
 {

-- 
2.56.0

Re: [PATCH bpf 1/2] bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect

[Alexei Starovoitov]

On Sun, Oct 25, 2026 at 04:41 PM Eli Skeggs via B4 Relay <devnull+kernel.eliskeggs.com@kernel.org> wrote:
> On an ingress redirect, the empty message is still queued as before, but
> the target socket no longer reports EPIPE.

That leaves the target broken.
Nothing dequeues the empty sk_msg. __sk_msg_recvmsg() sees
sge->length == 0, returns -EFAULT and leaves it at the head of ingress_msg.
tcp_bpf_recvmsg_parser() finds FIN in is_next_msg_fin(), returns 0 and
does copied_seq++ on a socket that didn't receive any FIN.
So every recv() on the target returns 0 and moves its copied_seq further
past rcv_nxt, and the data redirected from other sockets stays behind
that msg forever.

[...]

> -			if (ret <= 0) {
> +			if (ret < 0 || (!ret && len)) {

Lets skip empty skbs instead. That covers both directions.
The socket's own FIN that goes through the backlog is not queued either,
but SOCK_DONE check in tcp_bpf_recvmsg_parser() returns 0 anyway.

Something like this (compile tested only):

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index df385a5..a5003c7 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -711,7 +711,7 @@ static void sk_psock_backlog(struct work_struct *work)

 		ingress = skb_bpf_ingress(skb);
 		skb_bpf_redirect_clear(skb);
-		do {
+		while (len) {
 			ret = -EIO;
 			if (!sock_flag(psock->sk, SOCK_DEAD))
 				ret = sk_psock_handle_skb(psock, skb, off,
@@ -735,7 +735,7 @@ static void sk_psock_backlog(struct work_struct *work)
 			}
 			off += ret;
 			len -= ret;
-		} while (len);
+		}

 		/* The entire skb sent, clear state */
 		sk_psock_skb_state(psock, state, 0, 0);

pw-bot: cr

Re: [PATCH bpf 1/2] bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect

[Eli Skeggs]

On Mon, Oct 26, 2026 at 01:51:43AM +0000, Alexei Starovoitov wrote:
> On Sun, Oct 25, 2026 at 04:41 PM Eli Skeggs via B4 Relay <devnull+kernel.eliskeggs.com@kernel.org> wrote:
> > On an ingress redirect, the empty message is still queued as before, but
> > the target socket no longer reports EPIPE.
> 
> That leaves the target broken.
> Nothing dequeues the empty sk_msg. __sk_msg_recvmsg() sees
> sge->length == 0, returns -EFAULT and leaves it at the head of ingress_msg.
> tcp_bpf_recvmsg_parser() finds FIN in is_next_msg_fin(), returns 0 and
> does copied_seq++ on a socket that didn't receive any FIN.
> So every recv() on the target returns 0 and moves its copied_seq further
> past rcv_nxt, and the data redirected from other sockets stays behind
> that msg forever.

Thanks, I'd missed that detail of __sk_msg_recvmsg().

> Lets skip empty skbs instead. That covers both directions.
> The socket's own FIN that goes through the backlog is not queued either,
> but SOCK_DONE check in tcp_bpf_recvmsg_parser() returns 0 anyway.

That does sound much cleaner. v2 will skip empty skbs entirely, and I'll
add a corresponding selftest for the ingress pathway.

Re: [PATCH bpf 1/2] bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect

[Eli Skeggs]

On Mon, Oct 26, 2026 at 09:59:00AM +0800, Jiayuan Chen wrote:
>
> On 10/26/26 7:41 AM, Eli Skeggs via B4 Relay wrote:
> > From: Eli Skeggs <kernel@eliskeggs.com>
> >
> > Treat a return of 0 as success when there is no data to send, while
> > continuing to fail on a 0 return with residual bytes. The loop exits
> > without incident on an empty skb, and the skb gets cleaned up as usual.
> > On an ingress redirect, the empty message is still queued as before, but
> > the target socket no longer reports EPIPE.
>
> The target can't receive anything after that though. The empty msg sits
> at the head of psock->ingress_msg and nothing ever dequeues it:
>
> [...]
>
> So the user gets EOF on a socket whose peer never closed, and everything
> redirected to it after the FIN is lost. Without a verdict prog on the
> target it is worse, tcp_bpf_recvmsg() returns -EFAULT forever.
>
> Since this patch is already about the zero-length skb, can you handle
> the ingress side too? E.g. in sk_psock_skb_ingress(), don't queue an
> empty msg when len is 0 and the skb was redirected (skb->sk != sk). The
> self PASS case must keep its empty msg since is_next_msg_fin() depends
> on it.

Yep, thanks for pointing that out.

Alexei suggested skipping empty skbs in sk_psock_backlog():
https://lore.kernel.org/all/DMEE6SW3UE97.2DF11HBEBKYYI@gmail.com/

To your point, I think the critical difference between these approaches
happens when the sk's backlog queue is not empty and the socket's own
FIN (the self PASS FIN) gets queued. In that case, we rely on the empty
msg to correctly update copied_seq, and skipping it means the copied
sequence number ends up 1 behind rcv_nxt.

This path is already broken: when a socket's FIN gets queued behind
other skbs, the backlog reports the erroneous EPIPE just as in the
redirect case.

I do like the simplicity of skipping the empty skbs entirely. I don't
immediately see what the consequence is for copied_seq being behind in
this terminal state, but the approach that updates copied_seq does seem
strictly more correct.

Is that the right interpretation? Let me know what direction I should
take for v2.

Re: [PATCH bpf 1/2] bpf, sockmap: Stop reporting EPIPE on zero-length skb redirect

[Jiayuan Chen]

On 10/26/26 11:17 AM, Eli Skeggs wrote:
> On Mon, Oct 26, 2026 at 09:59:00AM +0800, Jiayuan Chen wrote:
>> On 10/26/26 7:41 AM, Eli Skeggs via B4 Relay wrote:
>>> From: Eli Skeggs <kernel@eliskeggs.com>
>>>
>>> Treat a return of 0 as success when there is no data to send, while
>>> continuing to fail on a 0 return with residual bytes. The loop exits
>>> without incident on an empty skb, and the skb gets cleaned up as usual.
>>> On an ingress redirect, the empty message is still queued as before, but
>>> the target socket no longer reports EPIPE.
>> The target can't receive anything after that though. The empty msg sits
>> at the head of psock->ingress_msg and nothing ever dequeues it:
>>
>> [...]
>>
>> So the user gets EOF on a socket whose peer never closed, and everything
>> redirected to it after the FIN is lost. Without a verdict prog on the
>> target it is worse, tcp_bpf_recvmsg() returns -EFAULT forever.
>>
>> Since this patch is already about the zero-length skb, can you handle
>> the ingress side too? E.g. in sk_psock_skb_ingress(), don't queue an
>> empty msg when len is 0 and the skb was redirected (skb->sk != sk). The
>> self PASS case must keep its empty msg since is_next_msg_fin() depends
>> on it.
> Yep, thanks for pointing that out.
>
> Alexei suggested skipping empty skbs in sk_psock_backlog():
> https://lore.kernel.org/all/DMEE6SW3UE97.2DF11HBEBKYYI@gmail.com/
>
> To your point, I think the critical difference between these approaches
> happens when the sk's backlog queue is not empty and the socket's own
> FIN (the self PASS FIN) gets queued. In that case, we rely on the empty
> msg to correctly update copied_seq, and skipping it means the copied
> sequence number ends up 1 behind rcv_nxt.
>
> This path is already broken: when a socket's FIN gets queued behind
> other skbs, the backlog reports the erroneous EPIPE just as in the
> redirect case.
>
> I do like the simplicity of skipping the empty skbs entirely. I don't
> immediately see what the consequence is for copied_seq being behind in
> this terminal state, but the approach that updates copied_seq does seem
> strictly more correct.
>
> Is that the right interpretation? Let me know what direction I should
> take for v2.


Yes, go with Alexei's suggestion. Skipping zero-length skbs in
sk_psock_backlog() also covers the ingress redirect case I raised,
since the empty msg is never queued. copied_seq being 1 behind
rcv_nxt is fine now, plain TCP sits in that state until the user reads
EOF, and nothing on the psock side depends on it.