chtls: Fix skb reference leak in chtls_send_reset()

[PATCH] chtls: Fix skb reference leak in chtls_send_reset()

Posted by Wentao Liang 1 week ago

chtls_send_reset() is given a skb holding one reference, to be sent as
CPL_ABORT_REQ. It consumes that reference on every path except the
TCP_SYN_RECV one: there it sends a TCB field reply built from a freshly
allocated skb and never releases the passed-in one.

This leaks the reference taken by reset_listen_child(), which resets
half-open children in TCP_SYN_RECV found on the SYN queue.

Release the skb on the TCP_SYN_RECV path as well.

Fixes: 8ad2a970d201 ("cxgb4/chtls: Fix tid stuck due to wrong update of qid")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
index 0e3e5cf52c2c..a363f20452fa 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
@@ -265,12 +265,14 @@ static void chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
 	chtls_purge_write_queue(sk);
 
 	csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
-	if (sk->sk_state != TCP_SYN_RECV)
+	if (sk->sk_state != TCP_SYN_RECV) {
 		chtls_send_abort(sk, mode, skb);
-	else
+	} else {
 		chtls_set_tcb_field_rpl_skb(sk, TCB_T_FLAGS_W,
 					    TCB_T_FLAGS_V(TCB_T_FLAGS_M), 0,
 					    TCB_FIELD_COOKIE_TFLAG, 1);
+		kfree_skb(skb);
+	}
 
 	return;
 out:
-- 
2.34.1

Re: [PATCH] chtls: Fix skb reference leak in chtls_send_reset()

Posted by Simon Horman 5 days, 18 hours ago

+ Sabrina

On Thu, Sep 17, 2026 at 10:58:46AM +0000, Wentao Liang wrote:
> chtls_send_reset() is given a skb holding one reference, to be sent as
> CPL_ABORT_REQ. It consumes that reference on every path except the
> TCP_SYN_RECV one: there it sends a TCB field reply built from a freshly
> allocated skb and never releases the passed-in one.
> 
> This leaks the reference taken by reset_listen_child(), which resets
> half-open children in TCP_SYN_RECV found on the SYN queue.
> 
> Release the skb on the TCP_SYN_RECV path as well.
> 
> Fixes: 8ad2a970d201 ("cxgb4/chtls: Fix tid stuck due to wrong update of qid")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> index 0e3e5cf52c2c..a363f20452fa 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> @@ -265,12 +265,14 @@ static void chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
>  	chtls_purge_write_queue(sk);
>  
>  	csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> -	if (sk->sk_state != TCP_SYN_RECV)
> +	if (sk->sk_state != TCP_SYN_RECV) {
>  		chtls_send_abort(sk, mode, skb);
> -	else
> +	} else {
>  		chtls_set_tcb_field_rpl_skb(sk, TCB_T_FLAGS_W,
>  					    TCB_T_FLAGS_V(TCB_T_FLAGS_M), 0,
>  					    TCB_FIELD_COOKIE_TFLAG, 1);
> +		kfree_skb(skb);
> +	}
>  
>  	return;
>  out:

This code change looks reasonable to me.
But the driver was removed in v7.2 by
cdae65fc43f2 ("tls: remove tls_toe and the related driver")
And, as a result it didn't run through our CI.

So while I think it could go to stable
It probably needs more eyes on it.

As an aside, I notice that this driver still appears
in MAINTAINERS in net-next. I will send a patch to remove it.

Re: [PATCH] chtls: Fix skb reference leak in chtls_send_reset()

Posted by Sabrina Dubroca 4 days, 9 hours ago

2026-09-19, 09:04:28 +0100, Simon Horman wrote:
> > diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > index 0e3e5cf52c2c..a363f20452fa 100644
> > --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > @@ -265,12 +265,14 @@ static void chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
> >  	chtls_purge_write_queue(sk);
> >  
> >  	csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> > -	if (sk->sk_state != TCP_SYN_RECV)
> > +	if (sk->sk_state != TCP_SYN_RECV) {
> >  		chtls_send_abort(sk, mode, skb);
> > -	else
> > +	} else {
> >  		chtls_set_tcb_field_rpl_skb(sk, TCB_T_FLAGS_W,
> >  					    TCB_T_FLAGS_V(TCB_T_FLAGS_M), 0,
> >  					    TCB_FIELD_COOKIE_TFLAG, 1);
> > +		kfree_skb(skb);
> > +	}
> >  
> >  	return;
> >  out:
> 
> This code change looks reasonable to me.
> But the driver was removed in v7.2 by
> cdae65fc43f2 ("tls: remove tls_toe and the related driver")
> And, as a result it didn't run through our CI.
> 
> So while I think it could go to stable
> It probably needs more eyes on it.

But there isn't anyone around that has even a remote understanding of
what this driver does... Since stable@ seems willing to take patches
that remove broken features [1], maybe we should just do that?

[1] https://lore.kernel.org/netdev/apbqjuv6l20bTjQI@laps/

-- 
Sabrina

Re: [PATCH] chtls: Fix skb reference leak in chtls_send_reset()

Posted by Simon Horman 3 days, 13 hours ago

On Sun, Sep 20, 2026 at 06:25:44PM +0200, Sabrina Dubroca wrote:
> 2026-09-19, 09:04:28 +0100, Simon Horman wrote:
> > > diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > > index 0e3e5cf52c2c..a363f20452fa 100644
> > > --- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > > +++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c
> > > @@ -265,12 +265,14 @@ static void chtls_send_reset(struct sock *sk, int mode, struct sk_buff *skb)
> > >  	chtls_purge_write_queue(sk);
> > >  
> > >  	csk_set_flag(csk, CSK_ABORT_SHUTDOWN);
> > > -	if (sk->sk_state != TCP_SYN_RECV)
> > > +	if (sk->sk_state != TCP_SYN_RECV) {
> > >  		chtls_send_abort(sk, mode, skb);
> > > -	else
> > > +	} else {
> > >  		chtls_set_tcb_field_rpl_skb(sk, TCB_T_FLAGS_W,
> > >  					    TCB_T_FLAGS_V(TCB_T_FLAGS_M), 0,
> > >  					    TCB_FIELD_COOKIE_TFLAG, 1);
> > > +		kfree_skb(skb);
> > > +	}
> > >  
> > >  	return;
> > >  out:
> > 
> > This code change looks reasonable to me.
> > But the driver was removed in v7.2 by
> > cdae65fc43f2 ("tls: remove tls_toe and the related driver")
> > And, as a result it didn't run through our CI.
> > 
> > So while I think it could go to stable
> > It probably needs more eyes on it.
> 
> But there isn't anyone around that has even a remote understanding of
> what this driver does... Since stable@ seems willing to take patches
> that remove broken features [1], maybe we should just do that?

Sure, that would address my concern regarding how to handle this quite nicely.

> 
> [1] https://lore.kernel.org/netdev/apbqjuv6l20bTjQI@laps/
> 
> -- 
> Sabrina