| Message ID | 20201118082107.7551-1-rohitm@chelsio.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ch_ktls: lock is not freed | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 12 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 2 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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 --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);
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(-)