| Message ID | 20201003111050.25130-1-alex.dewar90@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net/mlx5e: Fix freeing of unassigned pointer | expand |
On Sat, 2020-10-03 at 12:10 +0100, Alex Dewar wrote: > Commit ff7ea04ad579 ("net/mlx5e: Fix potential null pointer > dereference") > added some missing null checks but the error handling in > mlx5e_alloc_flow() was left broken: the variable attr is passed to > kfree > although it is never assigned to and never needs to be freed in this > function. Fix this. > > Addresses-Coverity-ID: 1497536 ("Memory - illegal accesses") > Fixes: ff7ea04ad579 ("net/mlx5e: Fix potential null pointer > dereference") > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 17 +++++++++---- > ---- > 1 file changed, 9 insertions(+), 8 deletions(-) > Hi Alex, thanks for the patch, Colin submitted a one liner patch that I already picked up. I hope you are ok with this. Thanks, Saeed.
On Tue, Oct 06, 2020 at 04:22:12PM -0700, Saeed Mahameed wrote: > On Sat, 2020-10-03 at 12:10 +0100, Alex Dewar wrote: > > Commit ff7ea04ad579 ("net/mlx5e: Fix potential null pointer > > dereference") > > added some missing null checks but the error handling in > > mlx5e_alloc_flow() was left broken: the variable attr is passed to > > kfree > > although it is never assigned to and never needs to be freed in this > > function. Fix this. > > > > Addresses-Coverity-ID: 1497536 ("Memory - illegal accesses") > > Fixes: ff7ea04ad579 ("net/mlx5e: Fix potential null pointer > > dereference") > > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > > --- > > drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 17 +++++++++---- > > ---- > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > Hi Alex, thanks for the patch, > Colin submitted a one liner patch that I already picked up. > > I hope you are ok with this. Hi Saeed, Sure. As long as attr is no longer being freed then that should fix the problem. Best, Alex > > Thanks, > Saeed. >
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c index a0c356987e1a..88298e96c4ea 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c @@ -4536,13 +4536,14 @@ mlx5e_alloc_flow(struct mlx5e_priv *priv, int attr_size, struct mlx5e_tc_flow_parse_attr *parse_attr; struct mlx5_flow_attr *attr; struct mlx5e_tc_flow *flow; - int err = -ENOMEM; int out_index; flow = kzalloc(sizeof(*flow), GFP_KERNEL); + if (!flow) + return -ENOMEM; parse_attr = kvzalloc(sizeof(*parse_attr), GFP_KERNEL); - if (!parse_attr || !flow) - goto err_free; + if (!parse_attr) + goto err_free_flow; flow->flags = flow_flags; flow->cookie = f->cookie; @@ -4550,7 +4551,7 @@ mlx5e_alloc_flow(struct mlx5e_priv *priv, int attr_size, attr = mlx5_alloc_flow_attr(get_flow_name_space(flow)); if (!attr) - goto err_free; + goto err_free_parse_attr; flow->attr = attr; @@ -4566,11 +4567,11 @@ mlx5e_alloc_flow(struct mlx5e_priv *priv, int attr_size, return 0; -err_free: - kfree(flow); +err_free_parse_attr: kvfree(parse_attr); - kfree(attr); - return err; +err_free_flow: + kfree(flow); + return -ENOMEM; } static void
Commit ff7ea04ad579 ("net/mlx5e: Fix potential null pointer dereference") added some missing null checks but the error handling in mlx5e_alloc_flow() was left broken: the variable attr is passed to kfree although it is never assigned to and never needs to be freed in this function. Fix this. Addresses-Coverity-ID: 1497536 ("Memory - illegal accesses") Fixes: ff7ea04ad579 ("net/mlx5e: Fix potential null pointer dereference") Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> --- drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)