* re: net/mlx5: Refactor tc flow attributes structure
@ 2020-09-28 16:06 Colin Ian King
2020-09-28 23:25 ` Saeed Mahameed
0 siblings, 1 reply; 2+ messages in thread
From: Colin Ian King @ 2020-09-28 16:06 UTC (permalink / raw)
To: Ariel Levkovich, Roi Dayan, Vlad Buslov, Saeed Mahameed
Cc: David S. Miller, netdev, linux-rdma, linux-kernel
Hi,
static analysis with Coverity has found a null pointer dereference issue
with the following commit:
commit c620b772152b8274031083bdb2e11c963e596c5c
Author: Ariel Levkovich <lariel@mellanox.com>
Date: Thu Apr 30 05:54:08 2020 +0300
net/mlx5: Refactor tc flow attributes structure
The analysis is as follows:
1240 slow_attr = mlx5_alloc_flow_attr(MLX5_FLOW_NAMESPACE_FDB);
1. Condition !slow_attr, taking true branch.
2. var_compare_op: Comparing slow_attr to null implies that
slow_attr might be null.
1241 if (!slow_attr)
1242 mlx5_core_warn(flow->priv->mdev, "Unable to
unoffload slow path rule\n");
1243
1244 memcpy(slow_attr, flow->attr, ESW_FLOW_ATTR_SZ);
Dereference after null check (FORWARD_NULL)
3. var_deref_op: Dereferencing null pointer slow_attr.
1245 slow_attr->action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
1246 slow_attr->esw_attr->split_count = 0;
1247 slow_attr->flags |= MLX5_ESW_ATTR_FLAG_SLOW_PATH;
1248 mlx5e_tc_unoffload_fdb_rules(esw, flow, slow_attr);
1249 flow_flag_clear(flow, SLOW);
1250 kfree(slow_attr);
there is a !slow_attr check but if it slow_attr is null the code then
dereferences it multiple times afterwards.
Colin
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: net/mlx5: Refactor tc flow attributes structure
2020-09-28 16:06 net/mlx5: Refactor tc flow attributes structure Colin Ian King
@ 2020-09-28 23:25 ` Saeed Mahameed
0 siblings, 0 replies; 2+ messages in thread
From: Saeed Mahameed @ 2020-09-28 23:25 UTC (permalink / raw)
To: Colin Ian King, Ariel Levkovich, Roi Dayan, Vlad Buslov, Saeed Mahameed
Cc: David S. Miller, netdev, linux-rdma, linux-kernel
On Mon, 2020-09-28 at 17:06 +0100, Colin Ian King wrote:
> Hi,
>
> static analysis with Coverity has found a null pointer dereference
> issue
> with the following commit:
>
> commit c620b772152b8274031083bdb2e11c963e596c5c
> Author: Ariel Levkovich <lariel@mellanox.com>
> Date: Thu Apr 30 05:54:08 2020 +0300
>
> net/mlx5: Refactor tc flow attributes structure
>
> The analysis is as follows:
>
> 1240 slow_attr =
> mlx5_alloc_flow_attr(MLX5_FLOW_NAMESPACE_FDB);
>
> 1. Condition !slow_attr, taking true branch.
> 2. var_compare_op: Comparing slow_attr to null implies that
> slow_attr might be null.
>
> 1241 if (!slow_attr)
> 1242 mlx5_core_warn(flow->priv->mdev, "Unable to
> unoffload slow path rule\n");
> 1243
> 1244 memcpy(slow_attr, flow->attr, ESW_FLOW_ATTR_SZ);
>
> Dereference after null check (FORWARD_NULL)
> 3. var_deref_op: Dereferencing null pointer slow_attr.
>
> 1245 slow_attr->action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> 1246 slow_attr->esw_attr->split_count = 0;
> 1247 slow_attr->flags |= MLX5_ESW_ATTR_FLAG_SLOW_PATH;
> 1248 mlx5e_tc_unoffload_fdb_rules(esw, flow, slow_attr);
> 1249 flow_flag_clear(flow, SLOW);
> 1250 kfree(slow_attr);
>
> there is a !slow_attr check but if it slow_attr is null the code then
> dereferences it multiple times afterwards.
>
> Colin
Thanks Colin for the Report,
Ariel is handling this internally and we will be posting the patch
soon.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-09-28 23:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 16:06 net/mlx5: Refactor tc flow attributes structure Colin Ian King
2020-09-28 23:25 ` Saeed Mahameed
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).