[PATCH net] tcp: fix data-race in tcp_recv_should_stop

[PATCH net] tcp: fix data-race in tcp_recv_should_stop

[Quanye Yang via B4 Relay]

From: Quanye Yang <quanyeyang@proton.me>

BUG: KCSAN: data-race in do_recvmmsg / mptcp_recvmsg

read-write (marked) to 0xffff8880134d391c of 4 bytes by task 2619 on cpu 1:
 instrument_atomic_read_write include/linux/instrumented.h:113 [inline]
 sock_error include/net/sock.h:2565 [inline]
 do_recvmmsg+0x50c/0x580 net/socket.c:3049
 __sys_recvmmsg net/socket.c:3144 [inline]
 __do_sys_recvmmsg net/socket.c:3167 [inline]
 __se_sys_recvmmsg net/socket.c:3160 [inline]
 __x64_sys_recvmmsg+0x161/0x180 net/socket.c:3160
 x64_sys_call+0x19c7/0x1ca0 arch/x86/include/generated/asm/syscalls_64.h:300
 do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
 do_syscall_64+0xde/0x3d0 arch/x86/entry/syscall_64.c:84
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

read to 0xffff8880134d391c of 4 bytes by task 2620 on cpu 0:
 tcp_recv_should_stop include/net/tcp.h:3086 [inline]
 mptcp_recvmsg+0x54d/0xd50 net/mptcp/protocol.c:2466
 inet_recvmsg+0x204/0x210 net/ipv4/af_inet.c:894
 sock_recvmsg_nosec net/socket.c:1151 [inline]
 sock_recvmsg+0x11a/0x140 net/socket.c:1173
 ____sys_recvmsg+0x14b/0x3c0 net/socket.c:2933
 ___sys_recvmsg+0x116/0x160 net/socket.c:2975
 __sys_recvmsg net/socket.c:3008 [inline]
 __do_sys_recvmsg net/socket.c:3014 [inline]
 __se_sys_recvmsg net/socket.c:3011 [inline]
 __x64_sys_recvmsg+0xeb/0x160 net/socket.c:3011
 x64_sys_call+0x1319/0x1ca0 arch/x86/include/generated/asm/syscalls_64.h:48
 do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
 do_syscall_64+0xde/0x3d0 arch/x86/entry/syscall_64.c:84
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

value changed: 0x0000006b -> 0x00000000

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 UID: 0 PID: 2620 Comm: syz.2.33 Not tainted 7.2.0-g39d4f32c5d53 #76 PREEMPT(full)
Hardware name: QEMU Ubuntu 26.04 PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1ubuntu1 04/01/2014

do_recvmmsg() calls sock_error() without the socket lock. sock_error()
clears sk_err with xchg(), which races with the unmarked load in
tcp_recv_should_stop(). The same race exists for plain TCP with
concurrent recvmmsg() readers; KCSAN reported it on an MPTCP socket
because mptcp_recvmsg() uses this helper.

Use READ_ONCE() for the lockless peek. No extra ordering is needed:
the value is only used to decide whether receiving should stop.
This does not consume sk_err; the check-then-sock_error() TOCTOU
on the no-data paths is a separate issue.

Fixes: eb477fdd6803 ("tcp: add recv_should_stop helper")
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
---
Link: https://lore.kernel.org/all/14749060-d011-41e7-9a4b-754eb5bd9d5b@redhat.com/
---
 include/net/tcp.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 436495ff2271..c61d8678eafd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -3082,7 +3082,8 @@ enum skb_drop_reason tcp_inbound_hash(struct sock *sk,
 
 static inline int tcp_recv_should_stop(struct sock *sk)
 {
-	return sk->sk_err ||
+	/* sk_err can be cleared locklessly by sock_error(). */
+	return READ_ONCE(sk->sk_err) ||
 	       sk->sk_state == TCP_CLOSE ||
 	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
 	       signal_pending(current);

---
base-commit: 38b6be101006d3e7af972999f45d4f1e8250587a
change-id: 20260908-mptcp-sk-err-net-7ff88ef05044

Best regards,
--  
Quanye Yang <quanyeyang@proton.me>

Re: [PATCH net] tcp: fix data-race in tcp_recv_should_stop

[Matthieu Baerts]

Hi Quanye,

Thank you for this patch.

On 08/09/2026 04:59, Quanye Yang via B4 Relay wrote:
> From: Quanye Yang <quanyeyang@proton.me>
> 
> BUG: KCSAN: data-race in do_recvmmsg / mptcp_recvmsg
> 
> read-write (marked) to 0xffff8880134d391c of 4 bytes by task 2619 on cpu 1:
>  instrument_atomic_read_write include/linux/instrumented.h:113 [inline]
>  sock_error include/net/sock.h:2565 [inline]
>  do_recvmmsg+0x50c/0x580 net/socket.c:3049
>  __sys_recvmmsg net/socket.c:3144 [inline]
>  __do_sys_recvmmsg net/socket.c:3167 [inline]
>  __se_sys_recvmmsg net/socket.c:3160 [inline]
>  __x64_sys_recvmmsg+0x161/0x180 net/socket.c:3160
>  x64_sys_call+0x19c7/0x1ca0 arch/x86/include/generated/asm/syscalls_64.h:300
>  do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
>  do_syscall_64+0xde/0x3d0 arch/x86/entry/syscall_64.c:84
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> read to 0xffff8880134d391c of 4 bytes by task 2620 on cpu 0:
>  tcp_recv_should_stop include/net/tcp.h:3086 [inline]
>  mptcp_recvmsg+0x54d/0xd50 net/mptcp/protocol.c:2466
>  inet_recvmsg+0x204/0x210 net/ipv4/af_inet.c:894
>  sock_recvmsg_nosec net/socket.c:1151 [inline]
>  sock_recvmsg+0x11a/0x140 net/socket.c:1173
>  ____sys_recvmsg+0x14b/0x3c0 net/socket.c:2933
>  ___sys_recvmsg+0x116/0x160 net/socket.c:2975
>  __sys_recvmsg net/socket.c:3008 [inline]
>  __do_sys_recvmsg net/socket.c:3014 [inline]
>  __se_sys_recvmsg net/socket.c:3011 [inline]
>  __x64_sys_recvmsg+0xeb/0x160 net/socket.c:3011
>  x64_sys_call+0x1319/0x1ca0 arch/x86/include/generated/asm/syscalls_64.h:48
>  do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
>  do_syscall_64+0xde/0x3d0 arch/x86/entry/syscall_64.c:84
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> value changed: 0x0000006b -> 0x00000000
> 
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 0 UID: 0 PID: 2620 Comm: syz.2.33 Not tainted 7.2.0-g39d4f32c5d53 #76 PREEMPT(full)
> Hardware name: QEMU Ubuntu 26.04 PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1ubuntu1 04/01/2014

Just to avoid some confusions, this has been reported by MPTCP syzkaller
instances, not by Syzbot. So just not to have people trying to get more
details on the wrong website:

Reported-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/632

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH net] tcp: fix data-race in tcp_recv_should_stop

[quanyeyang]

 > Just to avoid some confusions, this has been reported by MPTCP syzkaller
> instances, not by Syzbot. So just not to have people trying to get more
> details on the wrong website:
> 
Thanks Matt, I will add the Reported-by and Closes tags in v2.

Quanye
> Reported-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/632
> 
> Cheers,
> Matt
> --
> Sponsored by the NGI0 Core fund.
> 
>

Re: [PATCH net] tcp: fix data-race in tcp_recv_should_stop

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete data-race fix: only the sk_err load inside
  tcp_recv_should_stop() is annotated, while every caller of the helper…

--

tcp: fix data-race in tcp_recv_should_stop

This converts the sk_err load in the tcp_recv_should_stop() helper to
READ_ONCE(), since do_recvmmsg() calls sock_error() without the socket
lock and clears sk_err with xchg(). A KCSAN report on an MPTCP socket
is quoted in the changelog.

Link: https://lore.kernel.org/all/14749060-d011-41e7-9a4b-754eb5bd9d5b@redhat.com/

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 436495ff2271d..c61d8678eafd3 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -3082,7 +3082,8 @@ enum skb_drop_reason tcp_inbound_hash(struct sock *sk,
>  
>  static inline int tcp_recv_should_stop(struct sock *sk)
>  {
> -	return sk->sk_err ||
> +	/* sk_err can be cleared locklessly by sock_error(). */
> +	return READ_ONCE(sk->sk_err) ||
>  	       sk->sk_state == TCP_CLOSE ||
>  	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
>  	       signal_pending(current);

[Severity: High]
Does annotating only the helper close the race the changelog describes?
Every caller of tcp_recv_should_stop() still has a plain load of the same
field in the same wait loop, a few lines below the helper call.

net/mptcp/protocol.c:mptcp_recvmsg() - the function named in the quoted
report:

	if (copied) {
		if (tcp_recv_should_stop(sk) ||
		    !timeo)
			break;
	} else {
		if (sk->sk_err) {
			copied = sock_error(sk);
			break;
		}

net/ipv4/tcp.c:tcp_recvmsg_locked() has the identical pair:

		if (copied) {
			if (!timeo ||
			    tcp_recv_should_stop(sk))
				break;
		} else {
			if (sock_flag(sk, SOCK_DONE))
				break;

			if (sk->sk_err) {
				copied = sock_error(sk);
				break;
			}

The same unmarked load also remains in tcp_splice_read() and in
mptcp_splice_read(), which are the other two callers of the helper.

The racing writer is unchanged in all four cases:

net/socket.c:do_recvmmsg() {
	...
	if (likely(!(flags & MSG_ERRQUEUE))) {
		err = sock_error(sock->sk);
	...
}

which runs with no socket lock and does xchg(&sk->sk_err, 0) in
sock_error(). So a thread in recvmsg() on the copied == 0 path and a
second thread in recvmmsg() on the same socket still produce the marked
read-write versus unmarked read pair, and the splat looks like it would
just be re-reported from mptcp_recvmsg() and tcp_recvmsg_locked()
directly instead of from the inlined helper.

Would it make sense to convert those four sibling loads to READ_ONCE()
in the same patch?

The changelog only mentions the leftover problem as:

    This does not consume sk_err; the check-then-sock_error() TOCTOU
    on the no-data paths is a separate issue.

That describes a semantic check-then-act concern, which is distinct from
the plain concurrent load. Could the changelog say explicitly which
sk_err loads remain unmarked after this change?

Converting the sibling reads would also make the change applicable to
trees that predate the Fixes: eb477fdd6803 helper, where only those reads
exist.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-mptcp-sk-err-net-v1-1-da71aaec9afd%40proton.me