diff mbox series

[net] ch_ktls: lock is not freed

Message ID 20201118082107.7551-1-rohitm@chelsio.com
State New
Headers show
Series [net] ch_ktls: lock is not freed | expand

Commit Message

Rohit Maheshwari Nov. 18, 2020, 8:21 a.m. UTC
Currently lock gets freed only if timeout expires, but missed a
case when HW returns failure and goes for cleanup.

Fixes: efca3878a5fb ("ch_ktls: Issue if connection offload fails")
Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>
---
 .../net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c   | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Nov. 20, 2020, 6:36 p.m. UTC | #1
On Wed, 18 Nov 2020 13:51:07 +0530 Rohit Maheshwari wrote:
> Currently lock gets freed only if timeout expires, but missed a

> case when HW returns failure and goes for cleanup.

> 

> Fixes: efca3878a5fb ("ch_ktls: Issue if connection offload fails")

> Signed-off-by: Rohit Maheshwari <rohitm@chelsio.com>

> ---

>  .../net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c   | 5 +++--

>  1 file changed, 3 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c

> index c24485c0d512..1f521751666d 100644

> --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c

> +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c

> @@ -594,9 +594,10 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,

>  free_l2t:

>  	cxgb4_l2t_release(tx_info->l2te);

>  free_tx_info:

> -	if (tx_info->pending_close)

> +	if (tx_info->open_state)

>  		spin_unlock_bh(&tx_info->lock);

> -	else

> +

> +	if (!tx_info->pending_close)

>  		kvfree(tx_info);

>  out:

>  	atomic64_inc(&port_stats->ktls_tx_connection_fail);


Are you 100% sure about this fix? The code seems to be jumping to the
error handler with or without this lock held. E.g. on line 558.

Please release the lock before jumping, in the two places that hold it.
It's far less complicated and actually fewer LoC.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index c24485c0d512..1f521751666d 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -594,9 +594,10 @@  static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 free_l2t:
 	cxgb4_l2t_release(tx_info->l2te);
 free_tx_info:
-	if (tx_info->pending_close)
+	if (tx_info->open_state)
 		spin_unlock_bh(&tx_info->lock);
-	else
+
+	if (!tx_info->pending_close)
 		kvfree(tx_info);
 out:
 	atomic64_inc(&port_stats->ktls_tx_connection_fail);