All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiayuan Chen <jiayuan.chen@linux.dev>
To: Geliang Tang <geliang@kernel.org>,
	Jiayuan Chen <jiayuan.chen@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	Jakub Sitnicki <jakub@cloudflare.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>
Cc: Geliang Tang <tanggeliang@kylinos.cn>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH v2] bpf, sockmap: Fix self-redirect copied_seq double-counting
Date: Mon, 7 Sep 2026 18:52:38 +0800	[thread overview]
Message-ID: <74c45711-2e73-42d9-8598-064b50147dad@linux.dev> (raw)
In-Reply-To: <9857cc27cd1c5bb8778141263fd79b29298fe24a.camel@kernel.org>


On 9/7/26 6:19 PM, Geliang Tang wrote:
> Hi Jiayuan,
>
> On Mon, 2026-09-07 at 09:34 +0800, Jiayuan Chen wrote:
>> On 9/5/26 3:03 PM, 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 converting self-redirect verdict to __SK_PASS at the
>>> beginning of sk_psock_verdict_apply(). This bypasses the
>>> __SK_REDIRECT case entirely (which calls sk_psock_eat_skb), letting
>>> the __SK_PASS path queue the skb to the psock ingress queue. The
>>> data is then read via tcp_bpf_recvmsg_parser(), which advances
>>> copied_seq exactly once through copied_from_self. Cross-socket
>>> redirects continue through __SK_REDIRECT with sk_psock_eat_skb()
>>> unchanged.
>>>
>>> Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling
>>> copied_seq")
>>> Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
>>> Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>> ---
>>> v2:
>>>    - fixup the verdict as Jakub and Jiayuan suggested.
>>>
>>> v1:
>>>    -
>>> https://patchwork.kernel.org/project/netdevbpf/patch/b840c35fdfdf36e9fddedfa645b12699bc51aa34.1787968065.git.tanggeliang@kylinos.cn/
>>> ---
>>>    net/core/skmsg.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
>>> index 2521b643fa05..df385a5a961e 100644
>>> --- a/net/core/skmsg.c
>>> +++ b/net/core/skmsg.c
>>> @@ -1000,6 +1000,10 @@ static int sk_psock_verdict_apply(struct
>>> sk_psock *psock, struct sk_buff *skb,
>>>    	int err = 0;
>>>    	u32 len, off;
>>>    
>>> +	if (verdict == __SK_REDIRECT && skb_bpf_ingress(skb) &&
>>> +	    skb_bpf_redirect_fetch(skb) == psock->sk)
>>> +		verdict = __SK_PASS;
>>> +
>>>    	switch (verdict) {
>>>    	case __SK_PASS:
>>>    		err = -EIO;
>>
>> LGTM.
>>
>> Please target to bpf tree so that BPF CI cat capature it.
> Sorry, I forgot to add the prefix. It's been a while since I last sent
> a patch to the bpf mailing list. Just to confirm, I should use [PATCH
> bpf] instead of [PATCH bpf-next] for this fix, correct?


[PATCH bpf] is enough :)


>
> Thanks,
> -Geliang
>
>>
>> Thanks
>>

  reply	other threads:[~2026-09-07 10:52 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-05  7:03 [PATCH v2] bpf, sockmap: Fix self-redirect copied_seq double-counting Geliang Tang
2026-09-07  1:34 ` Jiayuan Chen
2026-09-07 10:19   ` Geliang Tang
2026-09-07 10:52     ` Jiayuan Chen [this message]
  -- strict thread matches above, loose matches on Subject: below --
2026-09-09 15:06 netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74c45711-2e73-42d9-8598-064b50147dad@linux.dev \
    --to=jiayuan.chen@linux.dev \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geliang@kernel.org \
    --cc=horms@kernel.org \
    --cc=jakub@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=tanggeliang@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.