[PATCH v2 0/2] DPDK Coverity issue 381631, 381646

[PATCH v2 0/2] DPDK Coverity issue 381631, 381646

[Hernan Vargas]

v2: Split code unrelated to coverity fix in separate commit.
v1: Upstream fix to 23.03 for potential issue of dereferencing a pointer before null check.

Hernan Vargas (2):
  baseband/acc: fix check after deref and dead code
  baseband/acc: add check for empty queue

 drivers/baseband/acc/rte_acc100_pmd.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.37.1

[PATCH v2 1/2] baseband/acc: fix check after deref and dead code

[Hernan Vargas]

Fix potential issue of dereferencing a pointer before null check.
Remove null check for value that could never be null.

Coverity issue: 381646, 381631
Fixes: 989dec301a9 ("baseband/acc100: add ring companion address")
Cc: stable@dpdk.org

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index 36f6fec5ad..baab459436 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -4106,8 +4106,6 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 	int ret, cbm;
 	struct rte_bbdev_enc_op *op;
 
-	if (q == NULL)
-		return 0;
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	if (unlikely(ops == 0))
 		return 0;
-- 
2.37.1

Re: [PATCH v2 1/2] baseband/acc: fix check after deref and dead code

[Maxime Coquelin]

On 2/8/23 03:00, Hernan Vargas wrote:
> Fix potential issue of dereferencing a pointer before null check.
> Remove null check for value that could never be null.
> 
> Coverity issue: 381646, 381631
> Fixes: 989dec301a9 ("baseband/acc100: add ring companion address")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index 36f6fec5ad..baab459436 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -4106,8 +4106,6 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
>   	int ret, cbm;
>   	struct rte_bbdev_enc_op *op;
>   
> -	if (q == NULL)
> -		return 0;
>   #ifdef RTE_LIBRTE_BBDEV_DEBUG
>   	if (unlikely(ops == 0))
>   		return 0;

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH v2 2/2] baseband/acc: add check for empty queue

[Hernan Vargas]

Add optimization to return early if there are no available descriptors
in ring to dequeue.

Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
 drivers/baseband/acc/rte_acc100_pmd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
index baab459436..9941754aa0 100644
--- a/drivers/baseband/acc/rte_acc100_pmd.c
+++ b/drivers/baseband/acc/rte_acc100_pmd.c
@@ -4106,10 +4106,9 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
 	int ret, cbm;
 	struct rte_bbdev_enc_op *op;
 
-#ifdef RTE_LIBRTE_BBDEV_DEBUG
-	if (unlikely(ops == 0))
+	if (avail == 0)
 		return 0;
-#endif
+
 	op = acc_op_tail(q, 0);
 	if (unlikely(ops == NULL || op == NULL))
 		return 0;
-- 
2.37.1

Re: [PATCH v2 2/2] baseband/acc: add check for empty queue

[Maxime Coquelin]

Hi Hernan,

On 2/8/23 03:00, Hernan Vargas wrote:
> Add optimization to return early if there are no available descriptors
> in ring to dequeue.
> 
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
>   drivers/baseband/acc/rte_acc100_pmd.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/baseband/acc/rte_acc100_pmd.c b/drivers/baseband/acc/rte_acc100_pmd.c
> index baab459436..9941754aa0 100644
> --- a/drivers/baseband/acc/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc/rte_acc100_pmd.c
> @@ -4106,10 +4106,9 @@ acc100_dequeue_ldpc_enc(struct rte_bbdev_queue_data *q_data,
>   	int ret, cbm;
>   	struct rte_bbdev_enc_op *op;
>   
> -#ifdef RTE_LIBRTE_BBDEV_DEBUG
> -	if (unlikely(ops == 0))
> +	if (avail == 0)
>   		return 0;
> -#endif
> +

Again, this is doing more than advertised.

The patch intent is about returning early if no available descriptors,
but here you also remove some debug checks.

At least please mention in in the commit message & title.

>   	op = acc_op_tail(q, 0);
>   	if (unlikely(ops == NULL || op == NULL))
>   		return 0;

Thanks,
Maxime