[dpdk-dev] [PATCH 1/4] regex/mlx5: fix size of setup constants

[dpdk-dev] [PATCH 1/4] regex/mlx5: fix size of setup constants

[Michael Baum]

The constant representing the size of the metadata is defined as a
unsigned int variable with 32-bit.
Similarly the constant representing the maximal output is also defined
as a unsigned int variable with 32-bit.

There is potentially overflowing expression when those constants are
evaluated using 32-bit arithmetic, and then used in a context that
expects an expression of type size_t (64 bits, unsigned).

Change the size of the above constants to 64-bit.

Fixes: 30d604bb1504 ("regex/mlx5: fix type of setup constants")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/regex/mlx5/mlx5_regex_fastpath.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_regex_fastpath.c b/drivers/regex/mlx5/mlx5_regex_fastpath.c
index b57e7d7794..3ef5e6c1eb 100644
--- a/drivers/regex/mlx5/mlx5_regex_fastpath.c
+++ b/drivers/regex/mlx5/mlx5_regex_fastpath.c
@@ -25,8 +25,8 @@
 #include "mlx5_regex.h"
 
 #define MLX5_REGEX_MAX_WQE_INDEX 0xffff
-#define MLX5_REGEX_METADATA_SIZE UINT32_C(64)
-#define MLX5_REGEX_MAX_OUTPUT RTE_BIT32(11)
+#define MLX5_REGEX_METADATA_SIZE UINT64_C(64)
+#define MLX5_REGEX_MAX_OUTPUT RTE_BIT64(11)
 #define MLX5_REGEX_WQE_CTRL_OFFSET 12
 #define MLX5_REGEX_WQE_METADATA_OFFSET 16
 #define MLX5_REGEX_WQE_GATHER_OFFSET 32
-- 
2.25.1

[dpdk-dev] [PATCH 2/4] compress/mlx5: fix constant size in QP creation

[Michael Baum]

The mlx5_compress_qp_setup function makes shifting to the numeric
constant 1, then sends it as a parameter to rte_calloc function.

The rte_calloc function expects to get size_t (64 bits, unsigned) and
instead gets a 32-bit variable, because the numeric constant size is a
32-bit.
In case the shift is greater than 32 the variable will lose its value
even though the function can get 64-bit argument.

Change the size of the numeric constant 1 to 64-bit.

Fixes: 8619fcd5161b ("compress/mlx5: support queue pair operations")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/compress/mlx5/mlx5_compress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/compress/mlx5/mlx5_compress.c b/drivers/compress/mlx5/mlx5_compress.c
index 80c564f10b..90d009c56b 100644
--- a/drivers/compress/mlx5/mlx5_compress.c
+++ b/drivers/compress/mlx5/mlx5_compress.c
@@ -209,7 +209,7 @@ mlx5_compress_qp_setup(struct rte_compressdev *dev, uint16_t qp_id,
 		return -rte_errno;
 	}
 	dev->data->queue_pairs[qp_id] = qp;
-	opaq_buf = rte_calloc(__func__, 1u << log_ops_n,
+	opaq_buf = rte_calloc(__func__, RTE_BIT64(log_ops_n),
 			      sizeof(struct mlx5_gga_compress_opaque),
 			      sizeof(struct mlx5_gga_compress_opaque));
 	if (opaq_buf == NULL) {
-- 
2.25.1

Re: [dpdk-dev] [PATCH 2/4] compress/mlx5: fix constant size in QP creation

[Matan Azrad]

From: Michael Baum:
> The mlx5_compress_qp_setup function makes shifting to the numeric
> constant 1, then sends it as a parameter to rte_calloc function.
> 
> The rte_calloc function expects to get size_t (64 bits, unsigned) and instead
> gets a 32-bit variable, because the numeric constant size is a 32-bit.
> In case the shift is greater than 32 the variable will lose its value even though
> the function can get 64-bit argument.
> 
> Change the size of the numeric constant 1 to 64-bit.
> 
> Fixes: 8619fcd5161b ("compress/mlx5: support queue pair operations")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] compress/mlx5: fix constant size in QP creation

[Thomas Monjalon]

01/06/2021 09:11, Michael Baum:
> The mlx5_compress_qp_setup function makes shifting to the numeric
> constant 1, then sends it as a parameter to rte_calloc function.
> 
> The rte_calloc function expects to get size_t (64 bits, unsigned) and

No on 32-bit systems, size_t is 32 bits.

> instead gets a 32-bit variable, because the numeric constant size is a
> 32-bit.

Most of the patches of this series say "constant" where it is a variable.

> In case the shift is greater than 32 the variable will lose its value
> even though the function can get 64-bit argument.
> 
> Change the size of the numeric constant 1 to 64-bit.
[...]
> -	opaq_buf = rte_calloc(__func__, 1u << log_ops_n,
> +	opaq_buf = rte_calloc(__func__, RTE_BIT64(log_ops_n),

Re: [dpdk-dev] [dpdk-stable] [PATCH 2/4] compress/mlx5: fix constant size in QP creation

[Michael Baum]

External email: Use caution opening links or attachments

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, June 23, 2021 9:48 AM
> To: Michael Baum <michaelba@nvidia.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Matan Azrad <matan@nvidia.com>;
> Raslan Darawsheh <rasland@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: Re: [dpdk-stable] [PATCH 2/4] compress/mlx5: fix constant size in
> QP creation
> 
> External email: Use caution opening links or attachments
> 
> 
> 01/06/2021 09:11, Michael Baum:
> > The mlx5_compress_qp_setup function makes shifting to the numeric
> > constant 1, then sends it as a parameter to rte_calloc function.
> >
> > The rte_calloc function expects to get size_t (64 bits, unsigned) and
> 
> No on 32-bit systems, size_t is 32 bits.

Thanks for the comment, I'll send v2.

> > instead gets a 32-bit variable, because the numeric constant size is a
> > 32-bit.
> 
> Most of the patches of this series say "constant" where it is a variable.
> 
> > In case the shift is greater than 32 the variable will lose its value
> > even though the function can get 64-bit argument.
> >
> > Change the size of the numeric constant 1 to 64-bit.
> [...]
> > -     opaq_buf = rte_calloc(__func__, 1u << log_ops_n,
> > +     opaq_buf = rte_calloc(__func__, RTE_BIT64(log_ops_n),
> 
> 

[dpdk-dev] [PATCH 3/4] vdpa/mlx5: fix constant type in QP creation

[Michael Baum]

The mlx5_vdpa_event_qp_create function makes shifting to the numeric
constant 1, then multiplies it by another constant and finally assigns
it into a uint64_t variable.

The numeric constant type is an int with a 32-bit sign. if after
shifting , its MSB (bit of sign) will change, the uint64 variable will
get into it a different value than what the function intended it to get.

Set the numeric constant 1 to be uint64_t in the first place.

Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/vdpa/mlx5/mlx5_vdpa_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
index 88f6a4256d..3541c652ce 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
@@ -629,8 +629,8 @@ mlx5_vdpa_event_qp_create(struct mlx5_vdpa_priv *priv, uint16_t desc_n,
 	attr.wq_umem_id = eqp->umem_obj->umem_id;
 	attr.wq_umem_offset = 0;
 	attr.dbr_umem_id = eqp->umem_obj->umem_id;
-	attr.dbr_address = (1 << log_desc_n) * MLX5_WSEG_SIZE;
 	attr.ts_format = mlx5_ts_format_conv(priv->qp_ts_format);
+	attr.dbr_address = RTE_BIT64(log_desc_n) * MLX5_WSEG_SIZE;
 	eqp->sw_qp = mlx5_devx_cmd_create_qp(priv->ctx, &attr);
 	if (!eqp->sw_qp) {
 		DRV_LOG(ERR, "Failed to create SW QP(%u).", rte_errno);
-- 
2.25.1

Re: [dpdk-dev] [PATCH 3/4] vdpa/mlx5: fix constant type in QP creation

[Matan Azrad]

From: Michael Baum:
> The mlx5_vdpa_event_qp_create function makes shifting to the numeric
> constant 1, then multiplies it by another constant and finally assigns it into a
> uint64_t variable.
> 
> The numeric constant type is an int with a 32-bit sign. if after shifting , its MSB
> (bit of sign) will change, the uint64 variable will get into it a different value
> than what the function intended it to get.
> 
> Set the numeric constant 1 to be uint64_t in the first place.
> 
> Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

[dpdk-dev] [PATCH 4/4] net/mlx5: fix constant type in MP allocation

[Michael Baum]

The mlx5_mprq_alloc_mp function makes shifting to the numeric constant
1, for sending it as a parameter to rte_mempool_create function.

The rte_mempool_create function expects to get void pointer (64 bits,
uintptr_t) and instead gets a 32-bit variable, because the numeric
constant size is a 32-bit.
In case the shift is greater than 32 the variable will lose its value
even though the function can get 64-bit argument.

Change the size of the numeric constant 1 to 64-bit.

Fixes: 3a22f3877c9d ("net/mlx5: replace external mbuf shared memory")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index bb9a908087..950f327f03 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1240,7 +1240,7 @@ mlx5_mprq_alloc_mp(struct rte_eth_dev *dev)
 	snprintf(name, sizeof(name), "port-%u-mprq", dev->data->port_id);
 	mp = rte_mempool_create(name, obj_num, obj_size, MLX5_MPRQ_MP_CACHE_SZ,
 				0, NULL, NULL, mlx5_mprq_buf_init,
-				(void *)(uintptr_t)(1 << strd_num_n),
+				(void *)(uintptr_t)RTE_BIT64(strd_num_n),
 				dev->device->numa_node, 0);
 	if (mp == NULL) {
 		DRV_LOG(ERR,
-- 
2.25.1

Re: [dpdk-dev] [PATCH 4/4] net/mlx5: fix constant type in MP allocation

[Matan Azrad]

From: Michael Baum:
> The mlx5_mprq_alloc_mp function makes shifting to the numeric constant 1,
> for sending it as a parameter to rte_mempool_create function.
> 
> The rte_mempool_create function expects to get void pointer (64 bits,
> uintptr_t) and instead gets a 32-bit variable, because the numeric constant
> size is a 32-bit.
> In case the shift is greater than 32 the variable will lose its value even though
> the function can get 64-bit argument.
> 
> Change the size of the numeric constant 1 to 64-bit.
> 
> Fixes: 3a22f3877c9d ("net/mlx5: replace external mbuf shared memory")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

Re: [dpdk-dev] [PATCH 1/4] regex/mlx5: fix size of setup constants

[Matan Azrad]

From: Michael Baum
> Sent: Tuesday, June 1, 2021 10:11 AM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH 1/4] regex/mlx5: fix size of setup constants
> 
> The constant representing the size of the metadata is defined as a unsigned
> int variable with 32-bit.
> Similarly the constant representing the maximal output is also defined as a
> unsigned int variable with 32-bit.
> 
> There is potentially overflowing expression when those constants are
> evaluated using 32-bit arithmetic, and then used in a context that expects an
> expression of type size_t (64 bits, unsigned).
> 
> Change the size of the above constants to 64-bit.
> 
> Fixes: 30d604bb1504 ("regex/mlx5: fix type of setup constants")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

[dpdk-dev] [PATCH v2 1/4] regex/mlx5: fix size of setup constants

[Michael Baum]

The constant representing the size of the metadata is defined as a
unsigned int variable with 32-bit.
Similarly the constant representing the maximal output is also defined
as a unsigned int variable with 32-bit.

There is potentially overflowing expression when those constants are
evaluated using 32-bit arithmetic, and then used in a context that
expects an expression of type size_t that might be 64 bit.

Change the size of the above constants to size_t.

Fixes: 30d604bb1504 ("regex/mlx5: fix type of setup constants")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
v2: use size_t and uintptr_t instead of uint64_t.

 drivers/regex/mlx5/mlx5_regex_fastpath.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regex/mlx5/mlx5_regex_fastpath.c b/drivers/regex/mlx5/mlx5_regex_fastpath.c
index b57e7d7794..910bc845f3 100644
--- a/drivers/regex/mlx5/mlx5_regex_fastpath.c
+++ b/drivers/regex/mlx5/mlx5_regex_fastpath.c
@@ -25,8 +25,8 @@
 #include "mlx5_regex.h"
 
 #define MLX5_REGEX_MAX_WQE_INDEX 0xffff
-#define MLX5_REGEX_METADATA_SIZE UINT32_C(64)
-#define MLX5_REGEX_MAX_OUTPUT RTE_BIT32(11)
+#define MLX5_REGEX_METADATA_SIZE ((size_t)64)
+#define MLX5_REGEX_MAX_OUTPUT (((size_t)1) << 11)
 #define MLX5_REGEX_WQE_CTRL_OFFSET 12
 #define MLX5_REGEX_WQE_METADATA_OFFSET 16
 #define MLX5_REGEX_WQE_GATHER_OFFSET 32
-- 
2.25.1

[dpdk-dev] [PATCH v2 2/4] compress/mlx5: fix constant size in QP creation

[Michael Baum]

The mlx5_compress_qp_setup function makes shifting to the numeric
constant 1, then sends it as a parameter to rte_calloc function.

The rte_calloc function expects to get size_t (might be 64 bit) and
instead gets a 32-bit variable, because the numeric constant size is a
32-bit.
In case the shift is greater than 32 bit and it 64-system, the variable
will lose its value even though the function can get 64-bit argument.

Change the size of the numeric constant 1 to size_t.

Fixes: 8619fcd5161b ("compress/mlx5: support queue pair operations")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/compress/mlx5/mlx5_compress.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/compress/mlx5/mlx5_compress.c b/drivers/compress/mlx5/mlx5_compress.c
index 80c564f10b..f09f71ecd6 100644
--- a/drivers/compress/mlx5/mlx5_compress.c
+++ b/drivers/compress/mlx5/mlx5_compress.c
@@ -209,7 +209,7 @@ mlx5_compress_qp_setup(struct rte_compressdev *dev, uint16_t qp_id,
 		return -rte_errno;
 	}
 	dev->data->queue_pairs[qp_id] = qp;
-	opaq_buf = rte_calloc(__func__, 1u << log_ops_n,
+	opaq_buf = rte_calloc(__func__, (size_t)1 << log_ops_n,
 			      sizeof(struct mlx5_gga_compress_opaque),
 			      sizeof(struct mlx5_gga_compress_opaque));
 	if (opaq_buf == NULL) {
-- 
2.25.1

[dpdk-dev] [PATCH v2 3/4] vdpa/mlx5: fix constant type in QP creation

[Michael Baum]

The mlx5_vdpa_event_qp_create function makes shifting to the numeric
constant 1, then multiplies it by another constant and finally assigns
it into a uint64_t variable.

The numeric constant type is an int with a 32-bit sign. if after
shifting , its MSB (bit of sign) will change, the uint64 variable will
get into it a different value than what the function intended it to get.

Set the numeric constant 1 to be uint64_t in the first place.

Fixes: 8395927cdfaf ("vdpa/mlx5: prepare HW queues")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/vdpa/mlx5/mlx5_vdpa_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/mlx5_vdpa_event.c b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
index 88f6a4256d..3541c652ce 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa_event.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa_event.c
@@ -629,8 +629,8 @@ mlx5_vdpa_event_qp_create(struct mlx5_vdpa_priv *priv, uint16_t desc_n,
 	attr.wq_umem_id = eqp->umem_obj->umem_id;
 	attr.wq_umem_offset = 0;
 	attr.dbr_umem_id = eqp->umem_obj->umem_id;
-	attr.dbr_address = (1 << log_desc_n) * MLX5_WSEG_SIZE;
 	attr.ts_format = mlx5_ts_format_conv(priv->qp_ts_format);
+	attr.dbr_address = RTE_BIT64(log_desc_n) * MLX5_WSEG_SIZE;
 	eqp->sw_qp = mlx5_devx_cmd_create_qp(priv->ctx, &attr);
 	if (!eqp->sw_qp) {
 		DRV_LOG(ERR, "Failed to create SW QP(%u).", rte_errno);
-- 
2.25.1

[dpdk-dev] [PATCH v2 4/4] net/mlx5: fix constant type in MP allocation

[Michael Baum]

The mlx5_mprq_alloc_mp function makes shifting to the numeric constant
1, for sending it as a parameter to rte_mempool_create function.

The rte_mempool_create function expects to get void pointer (uintptr_t,
might be 64-bit) and instead gets a 32-bit variable, because the
numeric constant size is a 32-bit.
In case the shift is greater than 32 the variable might lose its value
even though the function might get 64-bit argument.

Change the size of the numeric constant 1 to uintptr_t.

Fixes: 3a22f3877c9d ("net/mlx5: replace external mbuf shared memory")
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_rxq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index bb9a908087..83ee8ddb9c 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -1240,7 +1240,7 @@ mlx5_mprq_alloc_mp(struct rte_eth_dev *dev)
 	snprintf(name, sizeof(name), "port-%u-mprq", dev->data->port_id);
 	mp = rte_mempool_create(name, obj_num, obj_size, MLX5_MPRQ_MP_CACHE_SZ,
 				0, NULL, NULL, mlx5_mprq_buf_init,
-				(void *)(uintptr_t)(1 << strd_num_n),
+				(void *)((uintptr_t)1 << strd_num_n),
 				dev->device->numa_node, 0);
 	if (mp == NULL) {
 		DRV_LOG(ERR,
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2 1/4] regex/mlx5: fix size of setup constants

[Thomas Monjalon]

01/07/2021 08:39, Michael Baum:
> The constant representing the size of the metadata is defined as a
> unsigned int variable with 32-bit.
> Similarly the constant representing the maximal output is also defined
> as a unsigned int variable with 32-bit.
> 
> There is potentially overflowing expression when those constants are
> evaluated using 32-bit arithmetic, and then used in a context that
> expects an expression of type size_t that might be 64 bit.
> 
> Change the size of the above constants to size_t.
> 
> Fixes: 30d604bb1504 ("regex/mlx5: fix type of setup constants")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>

Series applied, thanks.