Re: [PATCH] tls: don't abort the connection on signal-interrupted sends
From: Maximilian Immanuel Brandtner <hidden>
Date: 2026-07-23 18:38:40
Also in:
lkml
On Thu, 2026-07-23 at 09:26 -0700, Jakub Kicinski wrote:
On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner wrote:quoted
When a signal interrupts a blocking send, tls_tx_records() treats the resulting -ERESTARTSYS as a transmission failure and marks the socket errored via tls_err_abort() with the raw error code. Later syscalls return the kernel-internal errno 512 (ERESTARTSYS) to userspace, as the signal it stems from is no longer pending during syscall exit and thus never translated.Can we just add ERESTARTSYS handling? I never heard of the other codes you're checking TBH, can they actually surface?
I don't know whether three are any drivers that call these error codes in practice. However, I know that these other errnos also get handled, by the signal re-entrancy logic (for example see `handle_signal()` in `arch/x86/kernel/signal.c`). I think all these error codes should propagate to the relevant signal logic to be converted to the right error code and avoid leaking kernel-internal error codes to user-space. Maybe EINTR could be removed from the patch. I just kept it in just in case.
quoted
An interrupted send is not a connection error: the partially sent record stays queued and is resent later. Interrupt error codes are therefore excluded from the abort in the same way as -EAGAIN. Fixes: b341ca51d267 ("tls: Fix tls_sw_sendmsg error handling") Signed-off-by: Maximilian Immanuel Brandtner <redacted> --- Tested on top of net commit 3f1f75536668 ("net: openvswitch: reject oversized nested action attrs"). Several subsystems contain static helper functions to classify these interrupt errnos. It might be worthwhile to refactor these static functions into a generic helper function.Yes, please.
I'll prepare a seperate patch-set for that then. Note this helper will only cover `ERESTARTSYS`, `ERESTARTNOINTR`, `ERESTARTNOHAND`, and `ERESTART_RESTARTBLOCK`, because there are a bunch of places in the kernel where these 4 error codes in particular are checked against.
quoted
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index d4afc90fd796..3c9e94069e82 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c@@ -405,6 +405,24 @@ static void tls_free_open_rec(struct sock *sk)} } +static bool tls_is_non_restartable_err(int err) +{ + if (err >= 0) + return false;This doesn't belong in a helper for classifying errors.
So you would prefer if (rc < 0 && !tls_is_restartable_err(rc))
quoted
+ switch (err) { + case -EAGAIN: + case -EINTR: + case -ERESTARTSYS: + case -ERESTARTNOINTR: + case -ERESTARTNOHAND: + case -ERESTART_RESTARTBLOCK: + return false; + default: + return true; + } +} + int tls_tx_records(struct sock *sk, int flags) { struct tls_context *tls_ctx = tls_get_ctx(sk);@@ -458,7 +476,7 @@ int tls_tx_records(struct sock *sk, int flags)} tx_err: - if (rc < 0 && rc != -EAGAIN) + if (tls_is_non_restartable_err(rc)) tls_err_abort(sk, rc); return rc;