| Message ID | 20240112072916.3726945-1-alexious@zju.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3] net/mlx5e: fix a double-free in arfs_create_groups | expand |
On Fri, Jan 12, 2024 at 03:29:16PM +0800, Zhipeng Lu wrote: > When `in` allocated by kvzalloc fails, arfs_create_groups will free > ft->g and return an error. However, arfs_create_table, the only caller of > arfs_create_groups, will hold this error and call to > mlx5e_destroy_flow_table, in which the ft->g will be freed again. > > Fixes: 1cabe6b0965e ("net/mlx5e: Create aRFS flow tables") > Signed-off-by: Zhipeng Lu <alexious@zju.edu.cn> Thanks, I think this is getting close. Can you please prepare a v4 with the nits below fixed? And please target at the 'net' tree, by making sure it is based on the main branch of that tree, and marking the subject as follows: Subject: [PATCH net v3] ... > --- > Changelog: > > v2: free ft->g just in arfs_create_groups with a unwind ladder. > v3: split the allocation of ft->g and in. Rename the error label. > remove some refector change in v2. > --- > .../net/ethernet/mellanox/mlx5/core/en_arfs.c | 26 +++++++++++-------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c > index bb7f86c993e5..0424ae068a60 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c > @@ -254,11 +254,13 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, > > ft->g = kcalloc(MLX5E_ARFS_NUM_GROUPS, > sizeof(*ft->g), GFP_KERNEL); > - in = kvzalloc(inlen, GFP_KERNEL); > - if (!in || !ft->g) { > - kfree(ft->g); > - kvfree(in); > + if(!ft->g) nit: (one) space after if, please > return -ENOMEM; > + > + in = kvzalloc(inlen, GFP_KERNEL); > + if (!in) { nit: one space is enough after if > + err = -ENOMEM; > + goto err_free_g; > } > > mc = MLX5_ADDR_OF(create_flow_group_in, in, match_criteria); ...
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c index bb7f86c993e5..0424ae068a60 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c @@ -254,11 +254,13 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, ft->g = kcalloc(MLX5E_ARFS_NUM_GROUPS, sizeof(*ft->g), GFP_KERNEL); - in = kvzalloc(inlen, GFP_KERNEL); - if (!in || !ft->g) { - kfree(ft->g); - kvfree(in); + if(!ft->g) return -ENOMEM; + + in = kvzalloc(inlen, GFP_KERNEL); + if (!in) { + err = -ENOMEM; + goto err_free_g; } mc = MLX5_ADDR_OF(create_flow_group_in, in, match_criteria); @@ -278,7 +280,7 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, break; default: err = -EINVAL; - goto out; + goto err_free_in; } switch (type) { @@ -300,7 +302,7 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, break; default: err = -EINVAL; - goto out; + goto err_free_in; } MLX5_SET_CFG(in, match_criteria_enable, MLX5_MATCH_OUTER_HEADERS); @@ -309,7 +311,7 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, MLX5_SET_CFG(in, end_flow_index, ix - 1); ft->g[ft->num_groups] = mlx5_create_flow_group(ft->t, in); if (IS_ERR(ft->g[ft->num_groups])) - goto err; + goto err_clean_group; ft->num_groups++; memset(in, 0, inlen); @@ -318,18 +320,20 @@ static int arfs_create_groups(struct mlx5e_flow_table *ft, MLX5_SET_CFG(in, end_flow_index, ix - 1); ft->g[ft->num_groups] = mlx5_create_flow_group(ft->t, in); if (IS_ERR(ft->g[ft->num_groups])) - goto err; + goto err_clean_group; ft->num_groups++; kvfree(in); return 0; -err: +err_clean_group: err = PTR_ERR(ft->g[ft->num_groups]); ft->g[ft->num_groups] = NULL; -out: +err_free_in: kvfree(in); - +err_free_g: + kfree(ft->g); + ft->g = NULL; return err; }
When `in` allocated by kvzalloc fails, arfs_create_groups will free ft->g and return an error. However, arfs_create_table, the only caller of arfs_create_groups, will hold this error and call to mlx5e_destroy_flow_table, in which the ft->g will be freed again. Fixes: 1cabe6b0965e ("net/mlx5e: Create aRFS flow tables") Signed-off-by: Zhipeng Lu <alexious@zju.edu.cn> --- Changelog: v2: free ft->g just in arfs_create_groups with a unwind ladder. v3: split the allocation of ft->g and in. Rename the error label. remove some refector change in v2. --- .../net/ethernet/mellanox/mlx5/core/en_arfs.c | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-)