* [PATCH] tls: don't abort the connection on signal-interrupted sends
@ 2026-07-20 9:08 Maximilian Immanuel Brandtner
2026-07-23 16:26 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Maximilian Immanuel Brandtner @ 2026-07-20 9:08 UTC (permalink / raw)
To: john.fastabend, kuba, sd, davem, edumazet, pabeni, horms,
bcodding, netdev, linux-kernel, svens, brueckner
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.
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 <maxbr@linux.ibm.com>
---
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.
---
net/tls/tls_sw.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
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;
+
+ 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;
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-20 9:08 [PATCH] tls: don't abort the connection on signal-interrupted sends Maximilian Immanuel Brandtner @ 2026-07-23 16:26 ` Jakub Kicinski 2026-07-23 18:38 ` Maximilian Immanuel Brandtner 2026-07-24 7:40 ` Paolo Abeni 0 siblings, 2 replies; 8+ messages in thread From: Jakub Kicinski @ 2026-07-23 16:26 UTC (permalink / raw) To: Maximilian Immanuel Brandtner Cc: john.fastabend, sd, davem, edumazet, pabeni, horms, bcodding, netdev, linux-kernel, svens, brueckner On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner wrote: > 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? > 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 <maxbr@linux.ibm.com> > --- > 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. > 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. > + 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; -- pw-bot: cr ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-23 16:26 ` Jakub Kicinski @ 2026-07-23 18:38 ` Maximilian Immanuel Brandtner 2026-07-23 18:44 ` Jakub Kicinski 2026-07-24 7:40 ` Paolo Abeni 1 sibling, 1 reply; 8+ messages in thread From: Maximilian Immanuel Brandtner @ 2026-07-23 18:38 UTC (permalink / raw) To: Jakub Kicinski Cc: john.fastabend, sd, davem, edumazet, pabeni, horms, bcodding, netdev, linux-kernel, svens, brueckner 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: > > 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. > > > 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 <maxbr@linux.ibm.com> > > --- > > 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. > > > 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)) > > > + 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; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-23 18:38 ` Maximilian Immanuel Brandtner @ 2026-07-23 18:44 ` Jakub Kicinski 0 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2026-07-23 18:44 UTC (permalink / raw) To: Maximilian Immanuel Brandtner Cc: john.fastabend, sd, davem, edumazet, pabeni, horms, bcodding, netdev, linux-kernel, svens, brueckner On Thu, 23 Jul 2026 20:38:27 +0200 Maximilian Immanuel Brandtner wrote: > > > +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)) Yup. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-23 16:26 ` Jakub Kicinski 2026-07-23 18:38 ` Maximilian Immanuel Brandtner @ 2026-07-24 7:40 ` Paolo Abeni 2026-07-25 3:09 ` Maximilian Immanuel Brandtner ` (2 more replies) 1 sibling, 3 replies; 8+ messages in thread From: Paolo Abeni @ 2026-07-24 7:40 UTC (permalink / raw) To: Jakub Kicinski, Maximilian Immanuel Brandtner Cc: john.fastabend, sd, davem, edumazet, horms, bcodding, netdev, linux-kernel, svens, brueckner On 7/23/26 6:26 PM, Jakub Kicinski wrote: > On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner wrote: >> 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? FTR, I think only EAGAIN and ERESTARTSYS can be observed in the network stack, and the latter only after sock_intr_errno() translation, i.e. on sendmsg()/recvmsg() return path. I would not add additional error code handling, until we have some proof that it's needed. /P ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-24 7:40 ` Paolo Abeni @ 2026-07-25 3:09 ` Maximilian Immanuel Brandtner 2026-07-25 3:39 ` Maximilian Immanuel Brandtner 2026-07-27 20:39 ` Maximilian Immanuel Brandtner 2 siblings, 0 replies; 8+ messages in thread From: Maximilian Immanuel Brandtner @ 2026-07-25 3:09 UTC (permalink / raw) To: Paolo Abeni, Jakub Kicinski Cc: john.fastabend, sd, davem, edumazet, horms, bcodding, netdev, linux-kernel, svens, brueckner On Fri, 2026-07-24 at 09:40 +0200, Paolo Abeni wrote: > On 7/23/26 6:26 PM, Jakub Kicinski wrote: > > On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner > > wrote: > > > 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? > > FTR, I think only EAGAIN and ERESTARTSYS can be observed in the > network > stack, and the latter only after sock_intr_errno() translation, i.e. > on > sendmsg()/recvmsg() return path. > > I would not add additional error code handling, until we have some > proof > that it's needed. > > /P Is this an explicit requirement in the subsystem or just a practical observation. Because if this is only fixed for ERESTARTSYS and some part of the networking subsystem later introduces code that might return these other restart error codes the same problem could occur again. By my lights, it's better to err on the side of caution, but if we can be certain that this assumption won't be broken in the future I'd be fine with dropping the other restart error codes. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-24 7:40 ` Paolo Abeni 2026-07-25 3:09 ` Maximilian Immanuel Brandtner @ 2026-07-25 3:39 ` Maximilian Immanuel Brandtner 2026-07-27 20:39 ` Maximilian Immanuel Brandtner 2 siblings, 0 replies; 8+ messages in thread From: Maximilian Immanuel Brandtner @ 2026-07-25 3:39 UTC (permalink / raw) To: Paolo Abeni, Jakub Kicinski Cc: john.fastabend, sd, davem, edumazet, horms, netdev, linux-kernel, svens, brueckner On Fri, 2026-07-24 at 09:40 +0200, Paolo Abeni wrote: > On 7/23/26 6:26 PM, Jakub Kicinski wrote: > > On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner > > wrote: > > > 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? > > FTR, I think only EAGAIN and ERESTARTSYS can be observed in the > network > stack, and the latter only after sock_intr_errno() translation, i.e. > on > sendmsg()/recvmsg() return path. > > I would not add additional error code handling, until we have somee > proof > that it's needed. > > /P > Also you may just have forgotten to mention it, but EINTR can also be returned from sock_intr_errno(), so at the very least the error codes EAGAIN, EINTR, ERESTARTSYS should be treated as non-fatal. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tls: don't abort the connection on signal-interrupted sends 2026-07-24 7:40 ` Paolo Abeni 2026-07-25 3:09 ` Maximilian Immanuel Brandtner 2026-07-25 3:39 ` Maximilian Immanuel Brandtner @ 2026-07-27 20:39 ` Maximilian Immanuel Brandtner 2 siblings, 0 replies; 8+ messages in thread From: Maximilian Immanuel Brandtner @ 2026-07-27 20:39 UTC (permalink / raw) To: Paolo Abeni, Jakub Kicinski Cc: john.fastabend, sd, davem, edumazet, horms, bcodding, netdev, linux-kernel, svens, brueckner On Fri, 2026-07-24 at 09:40 +0200, Paolo Abeni wrote: > On 7/23/26 6:26 PM, Jakub Kicinski wrote: > > On Mon, 20 Jul 2026 11:08:47 +0200 Maximilian Immanuel Brandtner > > wrote: > > > 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? > > FTR, I think only EAGAIN and ERESTARTSYS can be observed in the > network > stack, and the latter only after sock_intr_errno() translation, i.e. > on > sendmsg()/recvmsg() return path. > > I would not add additional error code handling, until we have some > proof > that it's needed. > > /P I have a v2 ready that only checks for EAGAIN, EINTR, and ERESTARTSYS. Before I send it though I would like to receive some feedback whether I should really remove the other error codes. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-27 21:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-20 9:08 [PATCH] tls: don't abort the connection on signal-interrupted sends Maximilian Immanuel Brandtner 2026-07-23 16:26 ` Jakub Kicinski 2026-07-23 18:38 ` Maximilian Immanuel Brandtner 2026-07-23 18:44 ` Jakub Kicinski 2026-07-24 7:40 ` Paolo Abeni 2026-07-25 3:09 ` Maximilian Immanuel Brandtner 2026-07-25 3:39 ` Maximilian Immanuel Brandtner 2026-07-27 20:39 ` Maximilian Immanuel Brandtner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox