On Sep 28, 2020, at 06:15, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Ariel Levkovich,
>
> The patch aedd133d17bc: "net/mlx5e: Support CT offload for tc nic
> flows" from Jul 21, 2020, leads to the following static checker
> warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:1132 mlx5e_tc_del_nic_flow()
> warn: passing freed memory 'flow'
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> 1105 static void mlx5e_tc_del_nic_flow(struct mlx5e_priv *priv,
> 1106 struct mlx5e_tc_flow *flow)
> 1107 {
> 1108 struct mlx5_flow_attr *attr = flow->attr;
> 1109 struct mlx5e_tc_table *tc = &priv->fs.tc;
> 1110
> 1111 flow_flag_clear(flow, OFFLOADED);
> 1112
> 1113 if (flow_flag_test(flow, CT))
> 1114 mlx5_tc_ct_delete_flow(get_ct_priv(flow->priv), flow, attr);
> ^^^^
> I guess this used to free "flow" and Smatch's db hasn't totally caught
> up yet. Now it doesn't use "flow" at all. Maybe we could just remove
> that parameter?
Hi Dan, thanks for pointing this out.
Yes, we can simply drop the flow and use priv directly as it is passed to the function as a separate variable.
Ariel
>
> 1115 else if (!IS_ERR_OR_NULL(flow->rule[0]))
> 1116 mlx5e_del_offloaded_nic_rule(priv, flow->rule[0], attr);
> 1117
> 1118 /* Remove root table if no rules are left to avoid
> 1119 * extra steering hops.
> 1120 */
> 1121 mutex_lock(&priv->fs.tc.t_lock);
> 1122 if (!mlx5e_tc_num_filters(priv, MLX5_TC_FLAG(NIC_OFFLOAD)) &&
> 1123 !IS_ERR_OR_NULL(tc->t)) {
> 1124 mlx5_chains_put_table(nic_chains(priv), 0, 1, MLX5E_TC_FT_LEVEL);
> 1125 priv->fs.tc.t = NULL;
> 1126 }
> 1127 mutex_unlock(&priv->fs.tc.t_lock);
> 1128
> 1129 kvfree(attr->parse_attr);
> 1130
> 1131 if (attr->action & MLX5_FLOW_CONTEXT_ACTION_MOD_HDR)
> 1132 mlx5e_detach_mod_hdr(priv, flow);
> ^^^^
> 1133
> 1134 mlx5_fc_destroy(priv->mdev, attr->counter);
> 1135
> 1136 if (flow_flag_test(flow, HAIRPIN))
> 1137 mlx5e_hairpin_flow_del(priv, flow);
> 1138
> 1139 kfree(flow->attr);
> 1140 }
>
> regards,
> dan carpenter
