[PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

When a BPF stream_verdict program redirects an skb back to the same
socket (self-redirect with BPF_F_INGRESS), sk_psock_verdict_apply()
calls tcp_eat_skb() which advances tcp_sk->copied_seq. However, the
skb is then delivered to the socket's psock ingress queue and later
read by tcp_bpf_recvmsg_parser(), which also advances copied_seq via
the copied_from_self accounting path. This double-counting causes
copied_seq to advance by 2x the actual data length, triggering:

  TCP recvmsg seq # bug 2: copied BF2E806, seq BF2E7FD, \
			   rcvnxt BF2E806, fl 0
  WARNING: net/ipv4/tcp.c:2745 at tcp_recvmsg_locked+0x72b/0x2640
  Call Trace:
   tcp_recvmsg+0x10a/0x500
   sock_recvmsg+0x168/0x1d0
   __sys_recvfrom+0x19a/0x2a0
   __x64_sys_recvfrom+0xe4/0x1f0
   do_syscall_64+0xf7/0x530
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

  cleanup rbuf bug: copied BF2E806 seq BF2E806 rcvnxt BF2E806
  WARNING: net/ipv4/tcp.c:1609 at tcp_cleanup_rbuf+0xf2/0x1c0
  Call Trace:
   tcp_recvmsg_locked+0x8d1/0x2640
   tcp_recvmsg+0x10a/0x500
   sock_recvmsg+0x168/0x1d0
   __sys_recvfrom+0x19a/0x2a0
   __x64_sys_recvfrom+0xe4/0x1f0
   do_syscall_64+0xf7/0x530
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fix this by checking if the redirect destination is the same socket.
For self-redirect (dst == psock->sk), skip tcp_eat_skb() since the
copied_seq will be advanced when the data is actually read from the
ingress queue. For cross-socket redirects, tcp_eat_skb() is still
needed to account for data leaving the source socket.

Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
Hi,

I encountered this while adding MPTCP BPF sockmap support. The existing
TCP sockmap selftests don't cover self-redirect, but the MPTCP tests do,
exposing this latent issue.

With this fix, both TCP and MPTCP tests pass, validating self-redirect
functionality.
---
 net/core/skmsg.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 2521b643fa05..5fa7b9639eef 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
 				goto out_free;
 		}
 		break;
-	case __SK_REDIRECT:
-		tcp_eat_skb(psock->sk, skb);
+	case __SK_REDIRECT: {
+		struct sock *dst = skb_bpf_redirect_fetch(skb);
+
+		if (dst != psock->sk)
+			tcp_eat_skb(psock->sk, skb);
 		err = sk_psock_skb_redirect(psock, skb);
 		break;
+	}
 	case __SK_DROP:
 	default:
 out_free:
-- 
2.53.0

Re: [PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally skipping tcp_eat_skb() for all self-redirects leaks sequence accounting (copied_seq) for egress redirects, synchronous redirect errors, and psock detach drops.
--

commit 8f3d3d5b6b4f4a55b1a1c00df40c2762471ef372
Author: Geliang Tang <geliang@kernel.org>

bpf, sockmap: Fix self-redirect copied_seq double-counting

This commit attempts to fix a double-counting issue in copied_seq when a
BPF stream_verdict program redirects an skb back to the same socket with
BPF_F_INGRESS. It does so by skipping tcp_eat_skb() for self-redirects.

> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 2521b643fa05d..5fa7b9639eefe 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
[ ... ]
> -	case __SK_REDIRECT:
> -		tcp_eat_skb(psock->sk, skb);
> +	case __SK_REDIRECT: {
> +		struct sock *dst = skb_bpf_redirect_fetch(skb);
> +
> +		if (dst != psock->sk)
> +			tcp_eat_skb(psock->sk, skb);
>  		err = sk_psock_skb_redirect(psock, skb);
>  		break;
> +	}
>  	case __SK_DROP:

[Severity: High]
Does conditionally skipping tcp_eat_skb() here leak sequence accounting
(copied_seq) if the self-redirect encounters an error or is an egress
redirect?

If a BPF program returns SK_REDIRECT to its own socket, tcp_eat_skb() is
bypassed. However, if this is an egress redirect (without BPF_F_INGRESS),
the packet is sent out via skb_send_sock() instead of being read by
tcp_bpf_recvmsg_parser().

Similarly, if sk_psock_skb_redirect() fails synchronously (for example, if
!sk_psock_test_state(psock_other, SK_PSOCK_TX_ENABLED)) and calls
sock_drop(), the packet is also lost.

In both cases, the packet is permanently removed from the ingress path
without advancing copied_seq. Could this cause a permanent sequence
desynchronization that stalls the receive window and triggers a kernel
warning in tcp_recvmsg_locked()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/b840c35fdfdf36e9fddedfa645b12699bc51aa34.1787968065.git.tanggeliang@kylinos.cn?part=1

Re: [PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[Jakub Sitnicki]

On Sat, Aug 29, 2026 at 10:00 AM +08, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> When a BPF stream_verdict program redirects an skb back to the same
> socket (self-redirect with BPF_F_INGRESS), sk_psock_verdict_apply()
> calls tcp_eat_skb() which advances tcp_sk->copied_seq. However, the
> skb is then delivered to the socket's psock ingress queue and later
> read by tcp_bpf_recvmsg_parser(), which also advances copied_seq via
> the copied_from_self accounting path. This double-counting causes
> copied_seq to advance by 2x the actual data length, triggering:
>
>   TCP recvmsg seq # bug 2: copied BF2E806, seq BF2E7FD, \
> 			   rcvnxt BF2E806, fl 0
>   WARNING: net/ipv4/tcp.c:2745 at tcp_recvmsg_locked+0x72b/0x2640
>   Call Trace:
>    tcp_recvmsg+0x10a/0x500
>    sock_recvmsg+0x168/0x1d0
>    __sys_recvfrom+0x19a/0x2a0
>    __x64_sys_recvfrom+0xe4/0x1f0
>    do_syscall_64+0xf7/0x530
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
>   cleanup rbuf bug: copied BF2E806 seq BF2E806 rcvnxt BF2E806
>   WARNING: net/ipv4/tcp.c:1609 at tcp_cleanup_rbuf+0xf2/0x1c0
>   Call Trace:
>    tcp_recvmsg_locked+0x8d1/0x2640
>    tcp_recvmsg+0x10a/0x500
>    sock_recvmsg+0x168/0x1d0
>    __sys_recvfrom+0x19a/0x2a0
>    __x64_sys_recvfrom+0xe4/0x1f0
>    do_syscall_64+0xf7/0x530
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Fix this by checking if the redirect destination is the same socket.
> For self-redirect (dst == psock->sk), skip tcp_eat_skb() since the
> copied_seq will be advanced when the data is actually read from the
> ingress queue. For cross-socket redirects, tcp_eat_skb() is still
> needed to account for data leaving the source socket.
>
> Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> Hi,
>
> I encountered this while adding MPTCP BPF sockmap support. The existing
> TCP sockmap selftests don't cover self-redirect, but the MPTCP tests do,
> exposing this latent issue.
>
> With this fix, both TCP and MPTCP tests pass, validating self-redirect
> functionality.
> ---
>  net/core/skmsg.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 2521b643fa05..5fa7b9639eef 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
>  				goto out_free;
>  		}
>  		break;
> -	case __SK_REDIRECT:
> -		tcp_eat_skb(psock->sk, skb);
> +	case __SK_REDIRECT: {
> +		struct sock *dst = skb_bpf_redirect_fetch(skb);
> +
> +		if (dst != psock->sk)
> +			tcp_eat_skb(psock->sk, skb);
>  		err = sk_psock_skb_redirect(psock, skb);
>  		break;
> +	}
>  	case __SK_DROP:
>  	default:
>  out_free:

Isn't the source of problem on the read-side (tcp_bpf_recvmsg_parser)?

We should be advancing copied_seq only for skbs that we received from
the tcp stack. That's why we have the copied_from_self detection in
tcp_bpf_recvmsg_parser.

I think the problem is that we set msg->sk when we call
sk_psock_skb_ingress_self from sk_psock_skb_ingress, so on SK_REDIRECT
path, not the SK_PASS path.

REDIRECT-to-self should really be a PASS, see [1]. My suggestion - fixup
the verdict:

        if (verdict == __SK_REDIRECT && skb->sk == psock->sk)
                verdict = __SK_PASS;

Then we can remove the sk_psock_skb_ingress_self call from
sk_psock_skb_ingress, and kill take_ref param in
sk_psock_skb_ingress_enqueue.

John, Jiayuan, thoughts?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2443ca66676d50a4eb3305c236bccd84a9828ce2

Re: [PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[Jiayuan Chen]

on 9/1/26 7:38 PM, Jakub Sitnicki wrote:
> On Sat, Aug 29, 2026 at 10:00 AM +08, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> When a BPF stream_verdict program redirects an skb back to the same
>> socket (self-redirect with BPF_F_INGRESS), sk_psock_verdict_apply()
>> calls tcp_eat_skb() which advances tcp_sk->copied_seq. However, the
>> skb is then delivered to the socket's psock ingress queue and later
>> read by tcp_bpf_recvmsg_parser(), which also advances copied_seq via

Hi Geliang,

tcp_eat_skb() will test 'skb_bpf_strparser(skb)' then skip the 
calculation of copied_seq.



>> the copied_from_self accounting path. This double-counting causes
>> copied_seq to advance by 2x the actual data length, triggering:
>>
>>    TCP recvmsg seq # bug 2: copied BF2E806, seq BF2E7FD, \
>> 			   rcvnxt BF2E806, fl 0
>>    WARNING: net/ipv4/tcp.c:2745 at tcp_recvmsg_locked+0x72b/0x2640
>>    Call Trace:
>>     tcp_recvmsg+0x10a/0x500
>>     sock_recvmsg+0x168/0x1d0
>>     __sys_recvfrom+0x19a/0x2a0
>>     __x64_sys_recvfrom+0xe4/0x1f0
>>     do_syscall_64+0xf7/0x530
>>     entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>
>>    cleanup rbuf bug: copied BF2E806 seq BF2E806 rcvnxt BF2E806
>>    WARNING: net/ipv4/tcp.c:1609 at tcp_cleanup_rbuf+0xf2/0x1c0
>>    Call Trace:
>>     tcp_recvmsg_locked+0x8d1/0x2640
>>     tcp_recvmsg+0x10a/0x500
>>     sock_recvmsg+0x168/0x1d0
>>     __sys_recvfrom+0x19a/0x2a0
>>     __x64_sys_recvfrom+0xe4/0x1f0
>>     do_syscall_64+0xf7/0x530
>>     entry_SYSCALL_64_after_hwframe+0x77/0x7f
>>
>> Fix this by checking if the redirect destination is the same socket.
>> For self-redirect (dst == psock->sk), skip tcp_eat_skb() since the
>> copied_seq will be advanced when the data is actually read from the
>> ingress queue. For cross-socket redirects, tcp_eat_skb() is still
>> needed to account for data leaving the source socket.
>>
>> Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>> ---
>> Hi,
>>
>> I encountered this while adding MPTCP BPF sockmap support. The existing
>> TCP sockmap selftests don't cover self-redirect, but the MPTCP tests do,
>> exposing this latent issue.
>>
>> With this fix, both TCP and MPTCP tests pass, validating self-redirect
>> functionality.
>> ---
>>   net/core/skmsg.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
>> index 2521b643fa05..5fa7b9639eef 100644
>> --- a/net/core/skmsg.c
>> +++ b/net/core/skmsg.c
>> @@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
>>   				goto out_free;
>>   		}
>>   		break;
>> -	case __SK_REDIRECT:
>> -		tcp_eat_skb(psock->sk, skb);
>> +	case __SK_REDIRECT: {
>> +		struct sock *dst = skb_bpf_redirect_fetch(skb);
>> +
>> +		if (dst != psock->sk)
>> +			tcp_eat_skb(psock->sk, skb);
>>   		err = sk_psock_skb_redirect(psock, skb);
>>   		break;
>> +	}
>>   	case __SK_DROP:
>>   	default:
>>   out_free:
> Isn't the source of problem on the read-side (tcp_bpf_recvmsg_parser)?


Right, I think I already fixed the parser side.


> We should be advancing copied_seq only for skbs that we received from
> the tcp stack. That's why we have the copied_from_self detection in
> tcp_bpf_recvmsg_parser.
>
> I think the problem is that we set msg->sk when we call
> sk_psock_skb_ingress_self from sk_psock_skb_ingress, so on SK_REDIRECT
> path, not the SK_PASS path.


I sucessfully use this selftest to reproduce the splat:

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c 
b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 1fef6ec2ba7a..58a90f2e3602 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1173,6 +1173,15 @@ static void test_sockmap_copied_seq(bool strp)
         if (!ASSERT_OK(err, "bpf_map_update_elem(p1)"))
                 goto end;

+       /* self redirect: data sent by c1 is redirected back to p1 itself */
+       sent = xsend(c1, buf, sizeof(buf), 0);
+       if (!ASSERT_EQ(sent, sizeof(buf), "xsend(c1), self"))
+               goto end;
+
+       recvd = recv_timeout(p1, rcv, sizeof(buf), MSG_DONTWAIT, 1);
+       if (!ASSERT_EQ(recvd, sent, "recv_timeout(p1), self"))
+               goto end;
+
         /* just trigger sockamp: data sent by c0 will be received by p1 */
         sent = xsend(c0, buf, sizeof(buf), 0);
         if (!ASSERT_EQ(sent, sizeof(buf), "xsend(c0), bpf"))
@@ -1364,6 +1373,8 @@ static void test_sockmap_no_verdict_fionread(void)

  void test_sockmap_basic(void)
  {
+       test_sockmap_copied_seq(false);
+       return;
         if (test__start_subtest("sockmap create_update_free"))
test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKMAP);
         if (test__start_subtest("sockhash create_update_free"))




> REDIRECT-to-self should really be a PASS, see [1]. My suggestion - fixup
> the verdict:
>
>          if (verdict == __SK_REDIRECT && skb->sk == psock->sk)
>                  verdict = __SK_PASS;


Agree. It's simple and clear.


+        if (verdict == __SK_REDIRECT && skb_bpf_ingress(skb) &&

+            skb_bpf_redirect_fetch(skb) == psock->sk)
+                verdict = __SK_PASS;

         switch (verdict) {


> Then we can remove the sk_psock_skb_ingress_self call from
> sk_psock_skb_ingress, and kill take_ref param in
> sk_psock_skb_ingress_enqueue.
>
> John, Jiayuan, thoughts?
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2443ca66676d50a4eb3305c236bccd84a9828ce2

Re: [PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 2 · Medium: 0 · Low: 0

- [High] net/core/skmsg.c:1042-1047 (sk_psock_verdict_apply(),
  __SK_REDIRECT arm): the new self-redirect test compares only the…
- [High] net/core/skmsg.c:1042-1047 (sk_psock_verdict_apply()): for a
  self-redirect the patch drops the up-front tcp_eat_skb() and relies on…

--

bpf, sockmap: Fix self-redirect copied_seq double-counting

This changes the __SK_REDIRECT arm of sk_psock_verdict_apply() so that
tcp_eat_skb() is only called when the redirect destination differs from
psock->sk.  The intent is to avoid advancing tcp_sk->copied_seq twice when
a stream_verdict program redirects an skb back into the same socket's
ingress queue.

> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 2521b643fa05..5fa7b9639eef 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
>  				goto out_free;
>  		}
>  		break;
> -	case __SK_REDIRECT:
> -		tcp_eat_skb(psock->sk, skb);
> +	case __SK_REDIRECT: {
> +		struct sock *dst = skb_bpf_redirect_fetch(skb);
> +
> +		if (dst != psock->sk)
> +			tcp_eat_skb(psock->sk, skb);

[Severity: High]
The commit message describes the problem as "self-redirect with
BPF_F_INGRESS", but this test only compares the destination socket and
never looks at the direction.  Does the same-socket *egress* case end up
here too?

skb_bpf_redirect_fetch() in include/linux/skmsg.h masks the flag bits off:

  static inline struct sock *skb_bpf_redirect_fetch(struct sk_buff *skb)
  {
	unsigned long sk_redir = skb->_sk_redir;

	return (struct sock *)(sk_redir & BPF_F_PTR_MASK);
  }

and bpf_sk_redirect_map() in net/core/sock_map.c stores the direction
separately, allowing flags == 0:

	if (unlikely(flags & ~(BPF_F_INGRESS)))
		return SK_DROP;
	...
	skb_bpf_set_redir(skb, sk, flags & BPF_F_INGRESS);

So a program doing bpf_sk_redirect_map(skb, &sock_map, idx, 0) where
sock_map[idx] resolves to the source socket satisfies dst == psock->sk and
skips tcp_eat_skb().

For that egress case nothing later advances copied_seq.  sk_psock_backlog()
computes:

		ingress = skb_bpf_ingress(skb);

which is false, so sk_psock_handle_skb() takes the transmit path:

		return skb_send_sock(psock->sk, skb, off, len);

and the skb is freed.  It never becomes an sk_msg with msg->sk == sk, so
the from_self accounting in __sk_msg_recvmsg():

		from_self = msg_rx->sk == sk;

never runs, and neither does the copied_from_self path in
tcp_bpf_recvmsg_parser() that the commit message relies on.

Meanwhile tcp_read_skb() already removed the bytes from the receive queue
without touching tp->copied_seq:

		__skb_unlink(skb, &sk->sk_receive_queue);

Can this leave tp->copied_seq permanently behind tp->rcv_nxt, so that
SIOCINQ reports data that no longer exists, __tcp_cleanup_rbuf() and
tcp_rcv_space_adjust() never run for those bytes, tcp_close() sees
data_was_unread != 0 and sends an RST instead of a FIN, and a later plain
tcp_recvmsg_locked() (after the psock is dropped) trips

	WARN(before(*seq, TCP_SKB_CB(skb)->seq), "TCP recvmsg seq # bug...

that is, the same copied_seq desync in the opposite direction?

Worth noting that the program cited as validation,
tools/testing/selftests/bpf/progs/mptcp_sockmap.c, uses exactly the egress
form:

	return bpf_sk_redirect_map(skb, &sock_map, redirect_idx, 0);

Would adding the direction to the condition cover this, e.g.

	if (dst != psock->sk || !skb_bpf_ingress(skb))
		tcp_eat_skb(psock->sk, skb);

[Severity: High]
A second question about the same condition: the accounting is now skipped
before the outcome of the redirect is known.  Is it guaranteed that the skb
really reaches ingress_msg and is read by tcp_bpf_recvmsg_parser()?

sk_psock_skb_redirect() has drop paths that free the skb and return -EIO
without any copied_seq update, and for a self-redirect psock_other is this
very same psock:

  net/core/skmsg.c:sk_psock_skb_redirect() {
	...
	if (!psock_other || sock_flag(sk_other, SOCK_DEAD)) {
		skb_bpf_redirect_clear(skb);
		sock_drop(from->sk, skb);
		return -EIO;
	}
	spin_lock_bh(&psock_other->ingress_lock);
	if (!sk_psock_test_state(psock_other, SK_PSOCK_TX_ENABLED)) {
		spin_unlock_bh(&psock_other->ingress_lock);
		skb_bpf_redirect_clear(skb);
		sock_drop(from->sk, skb);
		return -EIO;
	}
	...
  }

SK_PSOCK_TX_ENABLED can already be clear while sk->sk_data_ready is still
sk_psock_verdict_data_ready, because sk_psock_backlog() clears it on a hard
transmit/ingress error:

			if (ret == -EAGAIN) {
				...
			} else {
				sk_psock_clear_state(psock, SK_PSOCK_TX_ENABLED);
				...
			}

and sk_psock_stop() clears it during close/map-delete before the socket
backlog is drained.  The __SK_PASS arm just above tests the same two states
(SOCK_DEAD and !SK_PSOCK_TX_ENABLED), so they appear reachable at this
point.

Even after a successful skb_queue_tail(), skbs still on ingress_skb are
purged without accounting:

  net/core/skmsg.c:__sk_psock_zap_ingress() {
	while ((skb = skb_dequeue(&psock->ingress_skb)) != NULL) {
		skb_bpf_redirect_clear(skb);
		sock_drop(psock->sk, skb);
	}
	...
  }

and __sk_psock_purge_ingress_msg() frees queued sk_msgs the same way.

In all of these cases tcp_read_skb() has already unlinked the bytes from
sk->sk_receive_queue, and tcp_eat_skb() is the only place that catches up:

  net/ipv4/tcp_bpf.c:tcp_eat_skb() {
	copied = tcp->copied_seq + skb->len;
	WRITE_ONCE(tcp->copied_seq, copied);
	tcp_rcv_space_adjust(sk);
	__tcp_cleanup_rbuf(sk, skb->len);
  }

Note the sibling drop arm in this same switch keeps that contract:

  out_free:
		skb_bpf_redirect_clear(skb);
		tcp_eat_skb(psock->sk, skb);
		sock_drop(psock->sk, skb);

Should the skipped accounting be performed on the redirect-failure and
purge paths as well, or should tcp_eat_skb() only be skipped once the skb
is known to have been retained for same-socket ingress consumption?

>  		err = sk_psock_skb_redirect(psock, skb);
>  		break;
> +	}
>  	case __SK_DROP:
>  	default:
>  out_free:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/b840c35fdfdf36e9fddedfa645b12699bc51aa34.1787968065.git.tanggeliang%40kylinos.cn

Re: [PATCH] bpf, sockmap: Fix self-redirect copied_seq double-counting

[Geliang Tang]

Hi Jakub, Jiayuan,

On Tue, 2026-09-01 at 21:00 +0800, Jiayuan Chen wrote:
> 
> on 9/1/26 7:38 PM, Jakub Sitnicki wrote:
> > On Sat, Aug 29, 2026 at 10:00 AM +08, Geliang Tang wrote:
> > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > 
> > > When a BPF stream_verdict program redirects an skb back to the
> > > same
> > > socket (self-redirect with BPF_F_INGRESS),
> > > sk_psock_verdict_apply()
> > > calls tcp_eat_skb() which advances tcp_sk->copied_seq. However,
> > > the
> > > skb is then delivered to the socket's psock ingress queue and
> > > later
> > > read by tcp_bpf_recvmsg_parser(), which also advances copied_seq
> > > via
> 
> Hi Geliang,
> 
> tcp_eat_skb() will test 'skb_bpf_strparser(skb)' then skip the 
> calculation of copied_seq.
> 
> 
> 
> > > the copied_from_self accounting path. This double-counting causes
> > > copied_seq to advance by 2x the actual data length, triggering:
> > > 
> > >    TCP recvmsg seq # bug 2: copied BF2E806, seq BF2E7FD, \
> > > 			   rcvnxt BF2E806, fl 0
> > >    WARNING: net/ipv4/tcp.c:2745 at
> > > tcp_recvmsg_locked+0x72b/0x2640
> > >    Call Trace:
> > >     tcp_recvmsg+0x10a/0x500
> > >     sock_recvmsg+0x168/0x1d0
> > >     __sys_recvfrom+0x19a/0x2a0
> > >     __x64_sys_recvfrom+0xe4/0x1f0
> > >     do_syscall_64+0xf7/0x530
> > >     entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > 
> > >    cleanup rbuf bug: copied BF2E806 seq BF2E806 rcvnxt BF2E806
> > >    WARNING: net/ipv4/tcp.c:1609 at tcp_cleanup_rbuf+0xf2/0x1c0
> > >    Call Trace:
> > >     tcp_recvmsg_locked+0x8d1/0x2640
> > >     tcp_recvmsg+0x10a/0x500
> > >     sock_recvmsg+0x168/0x1d0
> > >     __sys_recvfrom+0x19a/0x2a0
> > >     __x64_sys_recvfrom+0xe4/0x1f0
> > >     do_syscall_64+0xf7/0x530
> > >     entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > 
> > > Fix this by checking if the redirect destination is the same
> > > socket.
> > > For self-redirect (dst == psock->sk), skip tcp_eat_skb() since
> > > the
> > > copied_seq will be advanced when the data is actually read from
> > > the
> > > ingress queue. For cross-socket redirects, tcp_eat_skb() is still
> > > needed to account for data leaving the source socket.
> > > 
> > > Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling
> > > copied_seq")
> > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > > ---
> > > Hi,
> > > 
> > > I encountered this while adding MPTCP BPF sockmap support. The
> > > existing
> > > TCP sockmap selftests don't cover self-redirect, but the MPTCP
> > > tests do,
> > > exposing this latent issue.
> > > 
> > > With this fix, both TCP and MPTCP tests pass, validating self-
> > > redirect
> > > functionality.
> > > ---
> > >   net/core/skmsg.c | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> > > index 2521b643fa05..5fa7b9639eef 100644
> > > --- a/net/core/skmsg.c
> > > +++ b/net/core/skmsg.c
> > > @@ -1039,10 +1039,14 @@ static int sk_psock_verdict_apply(struct
> > > sk_psock *psock, struct sk_buff *skb,
> > >   				goto out_free;
> > >   		}
> > >   		break;
> > > -	case __SK_REDIRECT:
> > > -		tcp_eat_skb(psock->sk, skb);
> > > +	case __SK_REDIRECT: {
> > > +		struct sock *dst = skb_bpf_redirect_fetch(skb);
> > > +
> > > +		if (dst != psock->sk)
> > > +			tcp_eat_skb(psock->sk, skb);
> > >   		err = sk_psock_skb_redirect(psock, skb);
> > >   		break;
> > > +	}
> > >   	case __SK_DROP:
> > >   	default:
> > >   out_free:
> > Isn't the source of problem on the read-side
> > (tcp_bpf_recvmsg_parser)?
> 
> 
> Right, I think I already fixed the parser side.
> 
> 
> > We should be advancing copied_seq only for skbs that we received
> > from
> > the tcp stack. That's why we have the copied_from_self detection in
> > tcp_bpf_recvmsg_parser.
> > 
> > I think the problem is that we set msg->sk when we call
> > sk_psock_skb_ingress_self from sk_psock_skb_ingress, so on
> > SK_REDIRECT
> > path, not the SK_PASS path.
> 
> 
> I sucessfully use this selftest to reproduce the splat:
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c 
> b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index 1fef6ec2ba7a..58a90f2e3602 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> @@ -1173,6 +1173,15 @@ static void test_sockmap_copied_seq(bool strp)
>          if (!ASSERT_OK(err, "bpf_map_update_elem(p1)"))
>                  goto end;
> 
> +       /* self redirect: data sent by c1 is redirected back to p1
> itself */
> +       sent = xsend(c1, buf, sizeof(buf), 0);
> +       if (!ASSERT_EQ(sent, sizeof(buf), "xsend(c1), self"))
> +               goto end;
> +
> +       recvd = recv_timeout(p1, rcv, sizeof(buf), MSG_DONTWAIT, 1);
> +       if (!ASSERT_EQ(recvd, sent, "recv_timeout(p1), self"))
> +               goto end;
> +
>          /* just trigger sockamp: data sent by c0 will be received by
> p1 */
>          sent = xsend(c0, buf, sizeof(buf), 0);
>          if (!ASSERT_EQ(sent, sizeof(buf), "xsend(c0), bpf"))
> @@ -1364,6 +1373,8 @@ static void
> test_sockmap_no_verdict_fionread(void)
> 
>   void test_sockmap_basic(void)
>   {
> +       test_sockmap_copied_seq(false);
> +       return;
>          if (test__start_subtest("sockmap create_update_free"))
> test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKMAP);
>          if (test__start_subtest("sockhash create_update_free"))
> 
> 
> 
> 
> > REDIRECT-to-self should really be a PASS, see [1]. My suggestion -
> > fixup
> > the verdict:
> > 
> >          if (verdict == __SK_REDIRECT && skb->sk == psock->sk)
> >                  verdict = __SK_PASS;
> 
> 
> Agree. It's simple and clear.
> 
> 
> +        if (verdict == __SK_REDIRECT && skb_bpf_ingress(skb) &&
> 
> +            skb_bpf_redirect_fetch(skb) == psock->sk)
> +                verdict = __SK_PASS;

Thank you for your suggestions. I will add both of your Suggested-by
tags in v2.

Sorry for the late reply. After applying this patch, some of my other
tests didn't pass. I spent a few days debugging and found that it was
an issue with the MPTCP implementation itself - specifically,
mptcp_read_skb(). It has now been resolved.

I've basically completed the MPTCP sockmap support on my side. I'll
send it out to the BPF mailing list for review when it's ready.

Thanks,
-Geliang

> 
>          switch (verdict) {
> 
> 
> > Then we can remove the sk_psock_skb_ingress_self call from
> > sk_psock_skb_ingress, and kill take_ref param in
> > sk_psock_skb_ingress_enqueue.
> > 
> > John, Jiayuan, thoughts?
> > 
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2443ca66676d50a4eb3305c236bccd84a9828ce2