On Wed, May 11, 2022 at 04:57:24PM +0300, Dan Carpenter wrote:
> [ I was reviewing old use after free warnings and stumbled across this
> one which still needs fixing - dan ]
>
> Hello Maor Gottlieb,
>
> The patch 6a48faeeca10: "net/mlx5: Add direct rule fs_cmd
> implementation" from Aug 20, 2019, leads to the following Smatch
> static checker warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/steering/fs_dr.c:53 set_miss_action()
> warn: 'action' was already freed.
>
> drivers/net/ethernet/mellanox/mlx5/core/steering/fs_dr.c
> 28 static int set_miss_action(struct mlx5_flow_root_namespace *ns,
> 29 struct mlx5_flow_table *ft,
> 30 struct mlx5_flow_table *next_ft)
> 31 {
> 32 struct mlx5dr_action *old_miss_action;
> 33 struct mlx5dr_action *action = NULL;
> 34 struct mlx5dr_table *next_tbl;
> 35 int err;
> 36
> 37 next_tbl = next_ft ? next_ft->fs_dr_table.dr_table : NULL;
> 38 if (next_tbl) {
> 39 action = mlx5dr_action_create_dest_table(next_tbl);
> 40 if (!action)
> 41 return -EINVAL;
> 42 }
> 43 old_miss_action = ft->fs_dr_table.miss_action;
> 44 err = mlx5dr_table_set_miss_action(ft->fs_dr_table.dr_table, action);
> 45 if (err && action) {
> 46 err = mlx5dr_action_destroy(action);
> 47 if (err) {
> 48 action = NULL;
> 49 mlx5_core_err(ns->dev, "Failed to destroy action (%d)\n",
> 50 err);
> 51 }
>
> If "err" is zero then "action" is freed.
>
> 52 }
> --> 53 ft->fs_dr_table.miss_action = action;
> ^^^^^^
> Use after free.
Thanks for the report.
https://lore.kernel.org/netdev/7fe70bbb120422cc71e6b018531954d58ea2e61e.1653397057.git.leonro@xxxxxxxxxx/T/#u
>
> 54 if (old_miss_action) {
> 55 err = mlx5dr_action_destroy(old_miss_action);
> 56 if (err)
> 57 mlx5_core_err(ns->dev, "Failed to destroy action (%d)\n",
> 58 err);
> 59 }
> 60
> 61 return err;
> 62 }
>
> regards,
> dan carpenter
