[dpdk-dev] [PATCH] crypto/nitrox: fix coverity defects

[dpdk-dev] [PATCH] crypto/nitrox: fix coverity defects

[Nagadheeraj Rottela]

Address the defects reported by coverity: Unintended sign extension
and Out-of-bounds access.

Coverity issue: 349899, 349905, 349911, 349921, 349923, 349926

Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")

Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
---
 drivers/crypto/nitrox/nitrox_csr.h | 18 +++++++++---------
 drivers/crypto/nitrox/nitrox_sym.c |  3 ++-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/nitrox/nitrox_csr.h b/drivers/crypto/nitrox/nitrox_csr.h
index 8cd92e38b..b4c969b26 100644
--- a/drivers/crypto/nitrox/nitrox_csr.h
+++ b/drivers/crypto/nitrox/nitrox_csr.h
@@ -12,15 +12,15 @@
 #define NITROX_CSR_ADDR(bar_addr, offset) (bar_addr + (offset))
 
 /* NPS packet registers */
-#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070 + ((_i) * 0x40000))
-#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CTLX(_i)		(0x10000 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CNTSX(_i)		(0x10008 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010 + ((_i) * 0x40000))
+#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CTLX(_i)		(0x10000UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CNTSX(_i)		(0x10008UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010UL + ((_i) * 0x40000UL))
 
 /* AQM Virtual Function Registers */
 #define AQMQ_QSZX(_i)			(0x20008 + ((_i)*0x40000))
diff --git a/drivers/crypto/nitrox/nitrox_sym.c b/drivers/crypto/nitrox/nitrox_sym.c
index 56410c44d..d1b32fec9 100644
--- a/drivers/crypto/nitrox/nitrox_sym.c
+++ b/drivers/crypto/nitrox/nitrox_sym.c
@@ -683,7 +683,8 @@ nitrox_sym_pmd_create(struct nitrox_device *ndev)
 	struct rte_cryptodev *cdev;
 
 	rte_pci_device_name(&ndev->pdev->addr, name, sizeof(name));
-	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN, "_n5sym");
+	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN - strlen(name),
+		 "_n5sym");
 	ndev->rte_sym_dev.driver = &nitrox_rte_sym_drv;
 	ndev->rte_sym_dev.numa_node = ndev->pdev->device.numa_node;
 	ndev->rte_sym_dev.devargs = NULL;
-- 
2.13.6

Re: [dpdk-dev] [PATCH] crypto/nitrox: fix coverity defects

[Akhil Goyal]

> 
> Address the defects reported by coverity: Unintended sign extension
> and Out-of-bounds access.
> 
> Coverity issue: 349899, 349905, 349911, 349921, 349923, 349926
> 
> Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
> Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")
> 
> Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> ---
Thomas,

Can you take this directly on master.

Thanks,
Akhil

Re: [dpdk-dev] [PATCH] crypto/nitrox: fix coverity defects

[Thomas Monjalon]

20/02/2020 12:07, Akhil Goyal:
> > 
> > Address the defects reported by coverity: Unintended sign extension
> > and Out-of-bounds access.
> > 
> > Coverity issue: 349899, 349905, 349911, 349921, 349923, 349926
> > 
> > Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
> > Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")
> > 
> > Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> > ---
> Thomas,
> 
> Can you take this directly on master.

No sorry, I can't because there is no review and test.

And I think we should not have general patches for Coverity defects.
Instead we must do one patch per issue. Here two:
	- sign of constants
	- out-of-bound access
In each patch, we must have a description of the consequence of the issue.

Thanks

[dpdk-dev] [PATCH v2 1/2] crypto/nitrox: fix invalid CSR register address generation

[Nagadheeraj Rottela]

If the NPS_PKT ring/port is greater than 8191 the NPS_PKT*() macros will
evaluate to incorrect values due to unintended sign extension from int
to unsigned long. To fix this, add UL suffix to the constants in these
macros.

Coverity issue: 349899, 349905, 349911, 349921, 349923

Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")

Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
---
 drivers/crypto/nitrox/nitrox_csr.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/nitrox/nitrox_csr.h b/drivers/crypto/nitrox/nitrox_csr.h
index 8cd92e38b..b4c969b26 100644
--- a/drivers/crypto/nitrox/nitrox_csr.h
+++ b/drivers/crypto/nitrox/nitrox_csr.h
@@ -12,15 +12,15 @@
 #define NITROX_CSR_ADDR(bar_addr, offset) (bar_addr + (offset))
 
 /* NPS packet registers */
-#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070 + ((_i) * 0x40000))
-#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CTLX(_i)		(0x10000 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CNTSX(_i)		(0x10008 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010 + ((_i) * 0x40000))
+#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CTLX(_i)		(0x10000UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CNTSX(_i)		(0x10008UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010UL + ((_i) * 0x40000UL))
 
 /* AQM Virtual Function Registers */
 #define AQMQ_QSZX(_i)			(0x20008 + ((_i)*0x40000))
-- 
2.13.6

[dpdk-dev] [PATCH v2 2/2] crypto/nitrox: fix array out of bounds access

[Nagadheeraj Rottela]

In nitrox_sym_pmd_create() the name array will overflow if the pci
device name is greater than 57 bytes. To fix this issue subtract pci
device name length from array length while appending substring to the
name.

Coverity issue: 349926

Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")

Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
---
 drivers/crypto/nitrox/nitrox_sym.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/nitrox/nitrox_sym.c b/drivers/crypto/nitrox/nitrox_sym.c
index 56410c44d..d1b32fec9 100644
--- a/drivers/crypto/nitrox/nitrox_sym.c
+++ b/drivers/crypto/nitrox/nitrox_sym.c
@@ -683,7 +683,8 @@ nitrox_sym_pmd_create(struct nitrox_device *ndev)
 	struct rte_cryptodev *cdev;
 
 	rte_pci_device_name(&ndev->pdev->addr, name, sizeof(name));
-	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN, "_n5sym");
+	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN - strlen(name),
+		 "_n5sym");
 	ndev->rte_sym_dev.driver = &nitrox_rte_sym_drv;
 	ndev->rte_sym_dev.numa_node = ndev->pdev->device.numa_node;
 	ndev->rte_sym_dev.devargs = NULL;
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2 1/2] crypto/nitrox: fix invalid CSR register address generation

[Akhil Goyal]

> 
> If the NPS_PKT ring/port is greater than 8191 the NPS_PKT*() macros will
> evaluate to incorrect values due to unintended sign extension from int
> to unsigned long. To fix this, add UL suffix to the constants in these
> macros.
> 
> Coverity issue: 349899, 349905, 349911, 349921, 349923
> 
> Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
> 
> Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> ---
>  drivers/crypto/nitrox/nitrox_csr.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/crypto/nitrox/nitrox_csr.h
> b/drivers/crypto/nitrox/nitrox_csr.h
> index 8cd92e38b..b4c969b26 100644
> --- a/drivers/crypto/nitrox/nitrox_csr.h
> +++ b/drivers/crypto/nitrox/nitrox_csr.h
> @@ -12,15 +12,15 @@
>  #define NITROX_CSR_ADDR(bar_addr, offset) (bar_addr + (offset))
> 
>  /* NPS packet registers */
> -#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060 + ((_i) * 0x40000))
> -#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068 + ((_i) * 0x40000))
> -#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070 + ((_i) * 0x40000))
> -#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080 + ((_i) * 0x40000))
> -#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078 + ((_i) * 0x40000))
> -#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088 + ((_i) * 0x40000))
> -#define NPS_PKT_SLC_CTLX(_i)		(0x10000 + ((_i) * 0x40000))
> -#define NPS_PKT_SLC_CNTSX(_i)		(0x10008 + ((_i) * 0x40000))
> -#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010 + ((_i) * 0x40000))
> +#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060UL + ((_i) * 0x40000UL))
> +#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068UL + ((_i) *
> 0x40000UL))
> +#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070UL + ((_i) * 0x40000UL))
> +#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080UL + ((_i) * 0x40000UL))
> +#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078UL + ((_i) *
> 0x40000UL))
> +#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088UL + ((_i) *
> 0x40000UL))
> +#define NPS_PKT_SLC_CTLX(_i)		(0x10000UL + ((_i) * 0x40000UL))
> +#define NPS_PKT_SLC_CNTSX(_i)		(0x10008UL + ((_i) *
> 0x40000UL))
> +#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010UL + ((_i) * 0x40000UL))
> 
>  /* AQM Virtual Function Registers */
>  #define AQMQ_QSZX(_i)			(0x20008 + ((_i)*0x40000))

AQMQ_QSZX may also need to be updated.

[dpdk-dev] [PATCH v3 1/2] crypto/nitrox: fix invalid CSR register address generation

[Nagadheeraj Rottela]

If the NPS_PKT ring/port is greater than 8191 the NPS_PKT*() macros will
evaluate to incorrect values due to unintended sign extension from int
to unsigned long. To fix this, add UL suffix to the constants in these
macros. The same problem is with AQMQ_QSZX() macro also.

Coverity issue: 349899, 349905, 349911, 349921, 349923

Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
Fixes: 0a8fc2423bff ("crypto/nitrox: introduce Nitrox driver")

Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
---
 drivers/crypto/nitrox/nitrox_csr.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/nitrox/nitrox_csr.h b/drivers/crypto/nitrox/nitrox_csr.h
index 8cd92e38b..de7a3c671 100644
--- a/drivers/crypto/nitrox/nitrox_csr.h
+++ b/drivers/crypto/nitrox/nitrox_csr.h
@@ -12,18 +12,18 @@
 #define NITROX_CSR_ADDR(bar_addr, offset) (bar_addr + (offset))
 
 /* NPS packet registers */
-#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070 + ((_i) * 0x40000))
-#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078 + ((_i) * 0x40000))
-#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CTLX(_i)		(0x10000 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_CNTSX(_i)		(0x10008 + ((_i) * 0x40000))
-#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010 + ((_i) * 0x40000))
+#define NPS_PKT_IN_INSTR_CTLX(_i)	(0x10060UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BADDRX(_i)	(0x10068UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_RSIZEX(_i)	(0x10070UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_DONE_CNTSX(_i)	(0x10080UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INSTR_BAOFF_DBELLX(_i)	(0x10078UL + ((_i) * 0x40000UL))
+#define NPS_PKT_IN_INT_LEVELSX(_i)		(0x10088UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CTLX(_i)		(0x10000UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_CNTSX(_i)		(0x10008UL + ((_i) * 0x40000UL))
+#define NPS_PKT_SLC_INT_LEVELSX(_i)	(0x10010UL + ((_i) * 0x40000UL))
 
 /* AQM Virtual Function Registers */
-#define AQMQ_QSZX(_i)			(0x20008 + ((_i)*0x40000))
+#define AQMQ_QSZX(_i)			(0x20008UL + ((_i) * 0x40000UL))
 
 static inline uint64_t
 nitrox_read_csr(uint8_t *bar_addr, uint64_t offset)
-- 
2.13.6

[dpdk-dev] [PATCH v3 2/2] crypto/nitrox: fix array out of bounds access

[Nagadheeraj Rottela]

In nitrox_sym_pmd_create() the name array will overflow if the pci
device name is greater than 57 bytes. To fix this issue subtract pci
device name length from array length while appending substring to the
name.

Coverity issue: 349926

Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")

Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
---
 drivers/crypto/nitrox/nitrox_sym.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/nitrox/nitrox_sym.c b/drivers/crypto/nitrox/nitrox_sym.c
index 56410c44d..d1b32fec9 100644
--- a/drivers/crypto/nitrox/nitrox_sym.c
+++ b/drivers/crypto/nitrox/nitrox_sym.c
@@ -683,7 +683,8 @@ nitrox_sym_pmd_create(struct nitrox_device *ndev)
 	struct rte_cryptodev *cdev;
 
 	rte_pci_device_name(&ndev->pdev->addr, name, sizeof(name));
-	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN, "_n5sym");
+	snprintf(name + strlen(name), RTE_CRYPTODEV_NAME_MAX_LEN - strlen(name),
+		 "_n5sym");
 	ndev->rte_sym_dev.driver = &nitrox_rte_sym_drv;
 	ndev->rte_sym_dev.numa_node = ndev->pdev->device.numa_node;
 	ndev->rte_sym_dev.devargs = NULL;
-- 
2.13.6

Re: [dpdk-dev] [PATCH v3 1/2] crypto/nitrox: fix invalid CSR register address generation

[Akhil Goyal]

> 
> If the NPS_PKT ring/port is greater than 8191 the NPS_PKT*() macros will
> evaluate to incorrect values due to unintended sign extension from int
> to unsigned long. To fix this, add UL suffix to the constants in these
> macros. The same problem is with AQMQ_QSZX() macro also.
> 
> Coverity issue: 349899, 349905, 349911, 349921, 349923
> 
> Fixes: 32e4930d5a3b ("crypto/nitrox: add hardware queue management")
> Fixes: 0a8fc2423bff ("crypto/nitrox: introduce Nitrox driver")
> 
> Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> ---

Cc:stable@dpdk.org

Applied to dpdk-next-crypto

Thanks.

Re: [dpdk-dev] [PATCH v3 2/2] crypto/nitrox: fix array out of bounds access

[Akhil Goyal]

> -----Original Message-----
> From: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> Sent: Friday, March 27, 2020 7:13 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; jsrikanth@marvell.com;
> Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> Subject: [PATCH v3 2/2] crypto/nitrox: fix array out of bounds access
> 
> In nitrox_sym_pmd_create() the name array will overflow if the pci
> device name is greater than 57 bytes. To fix this issue subtract pci
> device name length from array length while appending substring to the
> name.
> 
> Coverity issue: 349926
> 
> Fixes: 9fdef0cc2385 ("crypto/nitrox: create symmetric cryptodev")
> 
> Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> ---
Cc:stable@dpdk.org

Applied to dpdk-next-crypto

Thanks.