The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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; 4+ 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] 4+ 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
  0 siblings, 1 reply; 4+ 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] 4+ 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
  0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2026-07-23 18:45 UTC | newest]

Thread overview: 4+ 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox