* [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