[net] ch_ktls: lock is not freed

[net] ch_ktls: lock is not freed

[Rohit Maheshwari]

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);
-- 
2.18.1

Re: [net] ch_ktls: lock is not freed

[Jakub Kicinski]

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.