[PATCH net] tls: fix send() returning success after the TCP send failed

[PATCH net] tls: fix send() returning success after the TCP send failed

[Hengyu Liang]

Commit b341ca51d267 ("tls: Fix tls_sw_sendmsg error handling") made
tls_tx_records() pass the error of the TCP send to tls_err_abort()
instead of -EBADMSG.

However, bpf_exec_tx_verdict() still looks for EBADMSG in sk_err to
tell that the record was not sent. With any other error, the record
stays counted. As of now, send() on a kTLS socket returns success after
the TCP connection can no longer send, for example after
shutdown(SHUT_WR) or after a reset. Nothing reaches the peer, and the
caller is not told.

The issue can be reproduced with:

    python3 - <<'EOF'
    import socket, struct
    l = socket.create_server(("127.0.0.1", 0))
    c = socket.create_connection(l.getsockname())
    c.setsockopt(socket.SOL_TCP, 31, b"tls")
    c.setsockopt(282, 1, struct.pack("HH", 0x0303, 51) + bytes(36))
    c.shutdown(socket.SHUT_WR)
    print(c.send(b"hello"))
    EOF

Before commit b341ca51d267 ("tls: Fix tls_sw_sendmsg error handling"),
the result is:

    OSError: [Errno 74] Bad message

After that commit, the result is:

    5

This patch will make bpf_exec_tx_verdict() check for any error in
sk_err. The send() above then fails with EPIPE, as it does on a plain
TCP socket.

Fixes: b341ca51d267 ("tls: Fix tls_sw_sendmsg error handling")
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
Tested on net: with this patch, the send() above and a send() after a
reset fail with EPIPE, and a send() on a TCP_REPAIR socket fails with
EINVAL, as they do on a plain TCP socket. The tls selftests pass.

Not addressed here: a caller that ignores the error and sends again
still gets one more record queued per send(), because
sk_stream_error() clears sk_err for EPIPE. Before commit b341ca51d267
the EBADMSG stayed set and later sends failed early.

 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 226e0f176fdc..f3e496fa47ae 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -711,7 +711,7 @@ 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) {
+	if (err && err != -EINPROGRESS && sk->sk_err) {
 		*copied -= sk_msg_free(sk, msg);
 		tls_free_open_rec(sk);
 		err = -sk->sk_err;

base-commit: ead6fa9a0ab194f02a2f36d634b193e9c5f9a1c6
-- 
2.53.0

Re: [PATCH net] tls: fix send() returning success after the TCP send failed

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] tls: fix send() returning success after the TCP send failed

[Hengyu Liang]

Hi,

I found this while comparing the behaviour of different kernel versions:
a send() on a kTLS socket whose TCP socket is in repair mode fails with
EBADMSG on v6.8 but returns the full length on v6.18. Testing
b341ca51d267 and its parent showed the change comes from that commit,
and I then checked the shutdown(SHUT_WR) and reset cases from the
commit message as well.

Thanks,
Hengyu