net/mlx5: fix flow descriptor allocation in Direct Verbs mode.
Checks
Commit Message
Initialize flow descriptor tunnel member during flow creation.
Prevent access to stale data and pointers when flow descriptor is
reallocated after relase.
Fixes: 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")
Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
Comments
Hi Gregory,
>-----Original Message-----
>From: Gregory Etelson <getelson@nvidia.com>
>Sent: Monday, December 7, 2020 10:28 PM
>To: dev@dpdk.org
>Cc: Gregory Etelson <getelson@nvidia.com>; Matan Azrad
><matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Slava
>Ovsiienko <viacheslavo@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
>Xueming(Steven) Li <xuemingl@nvidia.com>
>Subject: [PATCH] net/mlx5: fix flow descriptor allocation in Direct Verbs mode.
>
>Initialize flow descriptor tunnel member during flow creation.
>Prevent access to stale data and pointers when flow descriptor is reallocated
>after relase.
>
>Fixes: 8bb81f2649b1 ("net/mlx5: use thread specific flow workspace")
The initialization issue is introduced in earlier patch.
>
>Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>---
> drivers/net/mlx5/mlx5_flow_dv.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
>b/drivers/net/mlx5/mlx5_flow_dv.c index aa21ff9613..c72f97e05f 100644
>--- a/drivers/net/mlx5/mlx5_flow_dv.c
>+++ b/drivers/net/mlx5/mlx5_flow_dv.c
>@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
> "not enough memory to create flow handle");
> return NULL;
> }
>- MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
>+ MLX5_ASSERT(wks->flow_idx - 1 < RTE_DIM(wks->flows));
wks->flow_idx is used as "next available index", should it be "wks->flow_idx < RTE_DIM(...)"?
> dev_flow = &wks->flows[wks->flow_idx++];
>+ memset(dev_flow, 0, sizeof(*dev_flow));
> dev_flow->handle = dev_handle;
> dev_flow->handle_idx = handle_idx;
> /*
>@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
> */
> dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
> MLX5_ST_SZ_BYTES(fte_match_set_misc4);
>- /*
>- * The matching value needs to be cleared to 0 before using. In the
>- * past, it will be automatically cleared when using rte_*alloc
>- * API. The time consumption will be almost the same as before.
>- */
>- memset(dev_flow->dv.value.buf, 0,
>MLX5_ST_SZ_BYTES(fte_match_param));
> dev_flow->ingress = attr->ingress;
> dev_flow->dv.transfer = attr->transfer;
> return dev_flow;
>--
>2.29.2
@@ -6232,8 +6232,9 @@ flow_dv_prepare(struct rte_eth_dev *dev,
"not enough memory to create flow handle");
return NULL;
}
- MLX5_ASSERT(wks->flow_idx + 1 < RTE_DIM(wks->flows));
+ MLX5_ASSERT(wks->flow_idx - 1 < RTE_DIM(wks->flows));
dev_flow = &wks->flows[wks->flow_idx++];
+ memset(dev_flow, 0, sizeof(*dev_flow));
dev_flow->handle = dev_handle;
dev_flow->handle_idx = handle_idx;
/*
@@ -6245,12 +6246,6 @@ flow_dv_prepare(struct rte_eth_dev *dev,
*/
dev_flow->dv.value.size = MLX5_ST_SZ_BYTES(fte_match_param) -
MLX5_ST_SZ_BYTES(fte_match_set_misc4);
- /*
- * The matching value needs to be cleared to 0 before using. In the
- * past, it will be automatically cleared when using rte_*alloc
- * API. The time consumption will be almost the same as before.
- */
- memset(dev_flow->dv.value.buf, 0, MLX5_ST_SZ_BYTES(fte_match_param));
dev_flow->ingress = attr->ingress;
dev_flow->dv.transfer = attr->transfer;
return dev_flow;