diff mbox series

[net-next] net/mlx5: Fix memory leak in IPsec RoCE creation

Message ID 1b414ea3a92aa0d07b6261cf641445f27bc619d8.1676811549.git.leon@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net/mlx5: Fix memory leak in IPsec RoCE creation | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 37 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Leon Romanovsky Feb. 19, 2023, 12:59 p.m. UTC 
From: Patrisious Haddad <phaddad@nvidia.com>

During IPsec RoCE TX creation a struct for the flow group creation is
allocated, but never freed. Free that struct once it is no longer in use.

Fixes: 22551e77e550 ("net/mlx5: Configure IPsec steering for egress RoCEv2 traffic")
Signed-off-by: Patrisious Haddad <phaddad@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Jakub Kicinski Feb. 21, 2023, 12:50 a.m. UTC | #1 
On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:
> -rule_fail:
> +fail_rule:
>  	mlx5_destroy_flow_group(roce->g);
> -fail:
> +fail_group:
>  	mlx5_destroy_flow_table(ft);
> +fail_table:
> +	kvfree(in);
>  	return err;

If you're touching all of them please name them after what they do.
Much easier to review.
Leon Romanovsky Feb. 21, 2023, 6:39 a.m. UTC | #2 
On Mon, Feb 20, 2023 at 04:50:00PM -0800, Jakub Kicinski wrote:
> On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:
> > -rule_fail:
> > +fail_rule:
> >  	mlx5_destroy_flow_group(roce->g);
> > -fail:
> > +fail_group:
> >  	mlx5_destroy_flow_table(ft);
> > +fail_table:
> > +	kvfree(in);
> >  	return err;
> 
> If you're touching all of them please name them after what they do.
> Much easier to review.

I can change it, but all mlx* drivers and randomly chosen place in ice
use label to show what fail and not what will be done. Such notation
gives an ability to refactor code without changing label names if
failed part of code is not removed.

Thanks
Jakub Kicinski Feb. 21, 2023, 4:30 p.m. UTC | #3 
On Tue, 21 Feb 2023 08:39:40 +0200 Leon Romanovsky wrote:
> On Mon, Feb 20, 2023 at 04:50:00PM -0800, Jakub Kicinski wrote:
> > On Sun, 19 Feb 2023 14:59:57 +0200 Leon Romanovsky wrote:  
> > > -rule_fail:
> > > +fail_rule:
> > >  	mlx5_destroy_flow_group(roce->g);
> > > -fail:
> > > +fail_group:
> > >  	mlx5_destroy_flow_table(ft);
> > > +fail_table:
> > > +	kvfree(in);
> > >  	return err;  
> > 
> > If you're touching all of them please name them after what they do.
> > Much easier to review.  
> 
> I can change it, but all mlx* drivers and randomly chosen place in ice
> use label to show what fail and not what will be done. Such notation
> gives an ability to refactor code without changing label names if
> failed part of code is not removed.

Please refactor, and it'd be great if the convention was changed for
all new code in these drivers.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
index 2c53589b765d..edc6798b359e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/ipsec_fs_roce.c
@@ -162,7 +162,7 @@  int mlx5_ipsec_fs_roce_tx_create(struct mlx5_core_dev *mdev,
 	if (IS_ERR(ft)) {
 		err = PTR_ERR(ft);
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx ft err=%d\n", err);
-		return err;
+		goto fail_table;
 	}
 
 	roce->ft = ft;
@@ -174,22 +174,25 @@  int mlx5_ipsec_fs_roce_tx_create(struct mlx5_core_dev *mdev,
 	if (IS_ERR(g)) {
 		err = PTR_ERR(g);
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx group err=%d\n", err);
-		goto fail;
+		goto fail_group;
 	}
 	roce->g = g;
 
 	err = ipsec_fs_roce_tx_rule_setup(mdev, roce, pol_ft);
 	if (err) {
 		mlx5_core_err(mdev, "Fail to create RoCE IPsec tx rules err=%d\n", err);
-		goto rule_fail;
+		goto fail_rule;
 	}
 
+	kvfree(in);
 	return 0;
 
-rule_fail:
+fail_rule:
 	mlx5_destroy_flow_group(roce->g);
-fail:
+fail_group:
 	mlx5_destroy_flow_table(ft);
+fail_table:
+	kvfree(in);
 	return err;
 }