[PATCH net-next 0/2] net: annotate remaining lockless sk->sk_err accesses

[PATCH net-next 0/2] net: annotate remaining lockless sk->sk_err accesses

[Quanye Yang via B4 Relay]

The TCP/MPTCP series that annotated lockless sk_err peeks and
consumes is in net-next. Paolo asked for the same treatment on
kTLS and on the unmarked writers that still race those readers.

do_recvmmsg() and getsockopt(SO_ERROR) still call sock_error()
without the socket lock. kTLS is a ULP on that same struct sock,
so tls_rx_rec_wait() has the same peek-versus-consume split, and
the send path still does a double unmarked load.

sock_dequeue_err_skb() can store sk_err from MSG_ERRQUEUE before
lock_sock(). strp_abort_strp() and sk_psock_report_error() write
the same field on the TCP/TLS and sockmap paths.

Patch 1 annotates the TLS readers and consumes sk_err once on the
no-data path. Patch 2 pairs the remaining writers with WRITE_ONCE().
No extra ordering is added; ICMP error-queue overwrite semantics
are unchanged.

Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/

Signed-off-by: Quanye Yang <quanyeyang@proton.me>
---
Quanye Yang (2):
      tls: annotate lockless access to sk->sk_err
      net: annotate lockless writes to sk->sk_err

 include/linux/skmsg.h     |  2 +-
 net/core/skbuff.c         |  5 +++--
 net/strparser/strparser.c |  2 +-
 net/tls/tls_device.c      |  5 +++--
 net/tls/tls_sw.c          | 45 ++++++++++++++++++++++++++++++---------------
 5 files changed, 38 insertions(+), 21 deletions(-)
---
base-commit: 071876fd50482a68603a9460d80dd6dd58827ee1
change-id: 20261002-tls-fix-sk-kcsan-err-6fef32744f15

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

[PATCH net-next 1/2] tls: annotate lockless access to sk->sk_err

[Quanye Yang via B4 Relay]

From: Quanye Yang <quanyeyang@proton.me>

kTLS sits on the same struct sock as TCP. do_recvmmsg() and
getsockopt(SO_ERROR) still call sock_error() without the socket lock
and clear sk_err with xchg().

tls_rx_rec_wait() already peeks when data has been copied and consumes
otherwise, but the outer if (sk_err) is an unmarked load. On the
no-data path that check-then-sock_error() window can return 0 after
another thread consumes the error. Call sock_error() once and only
return when it is non-zero; keep READ_ONCE() on the peek path.

tls_sw_sendmsg_locked(), tls_push_data() and bpf_exec_tx_verdict()
read sk_err twice. Fold those unmarked loads into one READ_ONCE()
and use that value as the returned errno. The field is still not
consumed there.

Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
---
 net/tls/tls_device.c |  5 +++--
 net/tls/tls_sw.c     | 45 ++++++++++++++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f11d0528fc43..03ce83a9d4e9 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -444,8 +444,9 @@ static int tls_push_data(struct sock *sk,
 	if ((flags & (MSG_MORE | MSG_EOR)) == (MSG_MORE | MSG_EOR))
 		return -EINVAL;
 
-	if (unlikely(sk->sk_err))
-		return -sk->sk_err;
+	rc = -READ_ONCE(sk->sk_err);
+	if (unlikely(rc))
+		return rc;
 
 	flags |= MSG_SENDPAGE_DECRYPTED;
 	tls_push_record_flags = flags | MSG_MORE;
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d1ad31986cf2..12e4458b44bb 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -704,10 +704,14 @@ static int bpf_exec_tx_verdict(struct sk_msg *msg, struct sock *sk,
 	int err;
 
 	err = tls_push_record(sk, flags, record_type);
-	if (err && err != -EINPROGRESS && sk->sk_err == EBADMSG) {
-		*copied -= sk_msg_free(sk, msg);
-		tls_free_open_rec(sk);
-		err = -sk->sk_err;
+	if (err && err != -EINPROGRESS) {
+		int skerr = READ_ONCE(sk->sk_err);
+
+		if (skerr == EBADMSG) {
+			*copied -= sk_msg_free(sk, msg);
+			tls_free_open_rec(sk);
+			err = -skerr;
+		}
 	}
 	return err;
 }
@@ -800,10 +804,9 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
 	}
 
 	while (msg_data_left(msg)) {
-		if (sk->sk_err) {
-			ret = -sk->sk_err;
+		ret = -READ_ONCE(sk->sk_err);
+		if (ret)
 			goto send_end;
-		}
 
 		if (ctx->open_rec)
 			rec = ctx->open_rec;
@@ -1107,10 +1110,16 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
 	timeo = sock_rcvtimeo(sk, nonblock);
 
 	while (!tls_strp_msg_ready(ctx)) {
-		if (sk->sk_err) {
-			if (has_copied)
-				return -READ_ONCE(sk->sk_err);
-			return sock_error(sk);
+		if (has_copied) {
+			int err = READ_ONCE(sk->sk_err);
+
+			if (err)
+				return -err;
+		} else {
+			int err = sock_error(sk);
+
+			if (err)
+				return err;
 		}
 
 		if (ret < 0)
@@ -1132,10 +1141,16 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
 		 * sk_err here so a connection abort surfaces as the
 		 * actual error rather than a clean EOF.
 		 */
-		if (sk->sk_err) {
-			if (has_copied)
-				return -READ_ONCE(sk->sk_err);
-			return sock_error(sk);
+		if (has_copied) {
+			int err = READ_ONCE(sk->sk_err);
+
+			if (err)
+				return -err;
+		} else {
+			int err = sock_error(sk);
+
+			if (err)
+				return err;
 		}
 		if (sk->sk_shutdown & RCV_SHUTDOWN)
 			return 0;

-- 
2.55.0

Re: [PATCH net-next 1/2] tls: annotate lockless access to sk->sk_err

[Eric Dumazet]

On Fri, Oct 2, 2026 at 9:29 AM Quanye Yang via B4 Relay
<devnull+quanyeyang.proton.me@kernel.org> wrote:
>
> From: Quanye Yang <quanyeyang@proton.me>
>
> kTLS sits on the same struct sock as TCP. do_recvmmsg() and
> getsockopt(SO_ERROR) still call sock_error() without the socket lock
> and clear sk_err with xchg().
>
> tls_rx_rec_wait() already peeks when data has been copied and consumes
> otherwise, but the outer if (sk_err) is an unmarked load. On the
> no-data path that check-then-sock_error() window can return 0 after
> another thread consumes the error. Call sock_error() once and only
> return when it is non-zero; keep READ_ONCE() on the peek path.
>
> tls_sw_sendmsg_locked(), tls_push_data() and bpf_exec_tx_verdict()
> read sk_err twice. Fold those unmarked loads into one READ_ONCE()
> and use that value as the returned errno. The field is still not
> consumed there.
>
> Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/
> Signed-off-by: Quanye Yang <quanyeyang@proton.me>
> ---
>  net/tls/tls_device.c |  5 +++--
>  net/tls/tls_sw.c     | 45 ++++++++++++++++++++++++++++++---------------
>  2 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index f11d0528fc43..03ce83a9d4e9 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -444,8 +444,9 @@ static int tls_push_data(struct sock *sk,
>         if ((flags & (MSG_MORE | MSG_EOR)) == (MSG_MORE | MSG_EOR))
>                 return -EINVAL;
>
> -       if (unlikely(sk->sk_err))
> -               return -sk->sk_err;
> +       rc = -READ_ONCE(sk->sk_err);
> +       if (unlikely(rc))
> +               return rc;
>
>         flags |= MSG_SENDPAGE_DECRYPTED;
>         tls_push_record_flags = flags | MSG_MORE;
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index d1ad31986cf2..12e4458b44bb 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

It seems you missed tls_encrypt_done() ?

Thanks!

Re: [PATCH net-next 1/2] tls: annotate lockless access to sk->sk_err

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [Low] The title calls this an annotation change, but the patch carries
  a user-visible behavioural fix and has no Fixes: tag.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the async
  receive epilogue of tls_sw_recvmsg() (net/tls/tls_sw.c) can overwrite…

--

tls: annotate lockless access to sk->sk_err

On the no-data path, tls_rx_rec_wait() now calls sock_error() once
instead of doing a plain sk_err check before it. The has_copied path
keeps using READ_ONCE(). In tls_sw_sendmsg_locked(), tls_push_data() and
bpf_exec_tx_verdict(), the two plain sk_err loads become a single
READ_ONCE() snapshot.

Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index d1ad31986cf2c..12e4458b44bb8 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -704,10 +704,14 @@ static int bpf_exec_tx_verdict(struct sk_msg *msg, struct sock *sk,
>  	int err;
>  
>  	err = tls_push_record(sk, flags, record_type);
> -	if (err && err != -EINPROGRESS && sk->sk_err == EBADMSG) {
> -		*copied -= sk_msg_free(sk, msg);
> -		tls_free_open_rec(sk);
> -		err = -sk->sk_err;
> +	if (err && err != -EINPROGRESS) {
> +		int skerr = READ_ONCE(sk->sk_err);
> +
> +		if (skerr == EBADMSG) {
> +			*copied -= sk_msg_free(sk, msg);
> +			tls_free_open_rec(sk);
> +			err = -skerr;
> +		}
>  	}
>  	return err;
>  }

[Severity: High]
bpf_exec_tx_verdict() now uses a single snapshot. Its async
counterpart, tls_encrypt_done(), still loads sk->sk_err three times
with plain loads. Is that intended?

net/tls/tls_sw.c:tls_encrypt_done() {
    ...
	/* Check if error is previously set on socket */
	if (err || sk->sk_err) {
		rec = NULL;

		/* If err is already set on socket, return the same code */
		if (sk->sk_err) {
			ctx->async_wait.err = -sk->sk_err;
		} else {
			ctx->async_wait.err = err;
			tls_err_abort(sk, err);
		}
	}
    ...
}

tls_encrypt_done() is the async AEAD completion callback. It runs from
the crypto engine, softirq or a kworker without the socket lock. The
racing side is the one the commit message names: sk_getsockopt() for
SO_ERROR does v.val = -sock_error(sk), which clears sk_err with xchg()
without the socket lock.

Suppose err is 0, the first check sees a non-zero sk_err, and the
xchg() clears it before the second check. The else branch then runs
with err == 0:

	ctx->async_wait.err = 0;
	tls_err_abort(sk, 0);

tls_err_abort() starts with WARN_ON_ONCE(err >= 0). Can this fire the
warning (or panic with panic_on_warn) and then write sk_err = 0?

In the other interleaving, the second check sees a non-zero value but
the -sk->sk_err load sees 0. async_wait.err becomes 0, and if err was
non-zero the real AEAD error is lost too. rec was already set to NULL,
so the record is never marked tx_ready. tls_encrypt_async_wait() then
returns 0 to the sender, and tls_tx_records() stops at the first record
that is not tx_ready.

After this patch, this looks like the last plain sk_err reader in
net/tls. The second patch in the series, "net: annotate lockless writes
to sk->sk_err", does not touch tls_encrypt_done() either.

Could this use one snapshot as well, for example
int skerr = READ_ONCE(sk->sk_err), for the test, the branch and the
assignment? Then tls_err_abort() would only ever be called with a
negative err.

[ ... ]

> @@ -1107,10 +1110,16 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
>  	timeo = sock_rcvtimeo(sk, nonblock);
>  
>  	while (!tls_strp_msg_ready(ctx)) {
> -		if (sk->sk_err) {
> -			if (has_copied)
> -				return -READ_ONCE(sk->sk_err);
> -			return sock_error(sk);
> +		if (has_copied) {
> +			int err = READ_ONCE(sk->sk_err);
> +
> +			if (err)
> +				return -err;
> +		} else {
> +			int err = sock_error(sk);
> +
> +			if (err)
> +				return err;
>  		}

[Severity: Low]
This isn't a bug, but the subject calls this an annotation change, and
this hunk changes user-visible behavior.

Before the patch, another thread could clear sk_err between the plain
check and sock_error(). The data_race(!sk->sk_err) fast path in
sock_error() then returned 0. With nothing copied, tls_sw_recvmsg()
passed that 0 to userspace as EOF:

	return copied ? : err;

The commit message describes this ("can return 0 after another thread
consumes the error"), but there is no Fixes: tag.

The first site comes from f508262ae9f2 ("tls: Preserve sk_err across
recvmsg() when data has been copied"), which replaced the older
return sock_error(sk) from 35560b7f06b8. The recheck after
sk_flush_backlog() below comes from edcf32b8a48f ("tls: Flush backlog
before waiting for a new record").

Should Fixes: tags be added so stable and backport tooling can pick up
the spurious EOF fix?

>  
>  		if (ret < 0)
> @@ -1132,10 +1141,16 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
>  		 * sk_err here so a connection abort surfaces as the
>  		 * actual error rather than a clean EOF.
>  		 */
> -		if (sk->sk_err) {
> -			if (has_copied)
> -				return -READ_ONCE(sk->sk_err);
> -			return sock_error(sk);
> +		if (has_copied) {
> +			int err = READ_ONCE(sk->sk_err);
> +
> +			if (err)
> +				return -err;
> +		} else {
> +			int err = sock_error(sk);
> +
> +			if (err)
> +				return err;
>  		}
>  		if (sk->sk_shutdown & RCV_SHUTDOWN)
>  			return 0;

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. Can
the async receive epilogue in tls_sw_recvmsg() overwrite a negative
return from tls_rx_rec_wait() with 0?

tls_rx_msg_size() only rejects data_len < cipher_overhead, so it
accepts a TLS 1.2 application-data record with empty plaintext. Take
ctx->async_capable with an AEAD that returns -EINPROGRESS:

tls_sw_recvmsg()
  tls_rx_one_record()          /* darg.async stays true */
  async |= darg.async;
  chunk = min_t(int, to_decrypt, len);     /* 0 */
  async_copy_bytes += chunk;               /* still 0 */
  tls_rx_rec_wait(..., has_copied=false)   /* -EAGAIN, or sock_error() */
recv_end:
  tls_decrypt_async_wait()                 /* 0 */
  err = process_rx_list(ctx, msg, &control, 0,
                        async_copy_bytes, is_peek, NULL);

The peek variant passes copied + peeked and decrypted - peeked, which
are also 0 here. With skip and len both 0, process_rx_list() skips both
of its loops and returns 0. That value overwrites err, and since copied
is 0, return copied ? : err returns 0.

On an open socket, would a non-blocking read return 0 (EOF) instead of
-EAGAIN? Would a connection error that sock_error() already consumed in
tls_rx_rec_wait() be lost and reported as a clean EOF?

Could the original receive error be kept when the async drain produces
no bytes and no error of its own?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002-tls-fix-sk-kcsan-err-v1-0-baa0ba056323%40proton.me

[PATCH net-next 2/2] net: annotate lockless writes to sk->sk_err

[Quanye Yang via B4 Relay]

From: Quanye Yang <quanyeyang@proton.me>

do_recvmmsg() and getsockopt(SO_ERROR) clear sk_err with xchg()
without the socket lock. TCP, MPTCP and kTLS already peek the same
field with READ_ONCE() or consume it via sock_error().

sock_dequeue_err_skb() still uses plain stores. tcp_recvmsg() can
call it via MSG_ERRQUEUE before lock_sock(), so those writes race
with the annotated readers and with sock_error(). The same unmarked
stores exist in strp_abort_strp() and sk_psock_report_error(), which
run on the TCP/TLS socket.

Annotate those writers with WRITE_ONCE(). No extra ordering is
needed; this does not change who wins when ICMP error-queue entries
overwrite sk_err.

Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/
Signed-off-by: Quanye Yang <quanyeyang@proton.me>
---
 include/linux/skmsg.h     | 2 +-
 net/core/skbuff.c         | 5 +++--
 net/strparser/strparser.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index d5e35f24738d..52ce45f25f5a 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -429,7 +429,7 @@ static inline void sk_psock_report_error(struct sk_psock *psock, int err)
 {
 	struct sock *sk = psock->sk;
 
-	sk->sk_err = err;
+	WRITE_ONCE(sk->sk_err, err);
 	sk_error_report(sk);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5c4024a03e10..51e3cf1ea985 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5535,12 +5535,13 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
 	if (skb && (skb_next = skb_peek(q))) {
 		icmp_next = is_icmp_err_skb(skb_next);
 		if (icmp_next)
-			sk->sk_err = SKB_EXT_ERR(skb_next)->ee.ee_errno;
+			WRITE_ONCE(sk->sk_err,
+				   SKB_EXT_ERR(skb_next)->ee.ee_errno);
 	}
 	spin_unlock_irqrestore(&q->lock, flags);
 
 	if (is_icmp_err_skb(skb) && !icmp_next)
-		sk->sk_err = 0;
+		WRITE_ONCE(sk->sk_err, 0);
 
 	if (skb_next)
 		sk_error_report(sk);
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index a23f4b4dfc67..e5d5d755e532 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -57,7 +57,7 @@ static void strp_abort_strp(struct strparser *strp, int err)
 		struct sock *sk = strp->sk;
 
 		/* Report an error on the lower socket */
-		sk->sk_err = -err;
+		WRITE_ONCE(sk->sk_err, -err);
 		sk_error_report(sk);
 	}
 }

-- 
2.55.0

Re: [PATCH net-next 2/2] net: annotate lockless writes to sk->sk_err

[Eric Dumazet]

On Fri, Oct 2, 2026 at 9:29 AM Quanye Yang via B4 Relay
<devnull+quanyeyang.proton.me@kernel.org> wrote:
>
> From: Quanye Yang <quanyeyang@proton.me>
>
> do_recvmmsg() and getsockopt(SO_ERROR) clear sk_err with xchg()
> without the socket lock. TCP, MPTCP and kTLS already peek the same
> field with READ_ONCE() or consume it via sock_error().
>
> sock_dequeue_err_skb() still uses plain stores. tcp_recvmsg() can
> call it via MSG_ERRQUEUE before lock_sock(), so those writes race
> with the annotated readers and with sock_error(). The same unmarked
> stores exist in strp_abort_strp() and sk_psock_report_error(), which
> run on the TCP/TLS socket.
>
> Annotate those writers with WRITE_ONCE(). No extra ordering is
> needed; this does not change who wins when ICMP error-queue entries
> overwrite sk_err.
>
> Link: https://lore.kernel.org/netdev/3d9d442f-f168-43da-87b0-010ad5a78365@redhat.com/
> Signed-off-by: Quanye Yang <quanyeyang@proton.me>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>