[dpdk-dev] net/vmxnet3: fix dereference before null check

Message ID 20170922130752.1908-1-michalx.k.jastrzebski@intel.com (mailing list archive)
State Rejected, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Michal Jastrzebski Sept. 22, 2017, 1:07 p.m. UTC
  From: Tomasz Kulasek <tomaszx.kulasek@intel.com>

Coverity error:

check_after_deref: Null-checking rq suggests that it may be null, but it
                   has already been dereferenced on all paths leading to
                   the check.

This patch moves NULL checking of "rq" at the very beginning of the path
before any dereference.

Coverity issue: 143468
Fixes: 5aecdc17a97d ("vmxnet3: fix stop/restart")
Cc: yongwang@vmware.com
Cc: stable@dpdk.org

Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
  

Comments

Michal Jastrzebski Sept. 22, 2017, 1:19 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Jastrzebski
> Sent: Friday, September 22, 2017 3:08 PM
> To: yliu@fridaylinux.org; maxime.coquelin@redhat.com
> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.jain@intel.com>; Kulasek,
> TomaszX <tomaszx.kulasek@intel.com>; yongwang@vmware.com;
> stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/vmxnet3: fix dereference before null check
> 
> From: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> 
> Coverity error:
> 
> check_after_deref: Null-checking rq suggests that it may be null, but it
>                    has already been dereferenced on all paths leading to
>                    the check.
> 
> This patch moves NULL checking of "rq" at the very beginning of the path
> before any dereference.
> 
> Coverity issue: 143468
> Fixes: 5aecdc17a97d ("vmxnet3: fix stop/restart")
> Cc: yongwang@vmware.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index d9cf437..4fcceb4 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -259,17 +259,16 @@
>  {
>  	int i;
>  	vmxnet3_rx_queue_t *rq = rxq;
> -	struct vmxnet3_hw *hw = rq->hw;
>  	struct vmxnet3_cmd_ring *ring0, *ring1;
>  	struct vmxnet3_comp_ring *comp_ring;
> -	struct vmxnet3_rx_data_ring *data_ring = &rq->data_ring;
>  	int size;
> 
> -	if (rq != NULL) {
> -		/* Release both the cmd_rings mbufs */
> -		for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++)
> -			vmxnet3_rx_cmd_ring_release_mbufs(&rq-
> >cmd_ring[i]);
> -	}
> +	if (rq == NULL)
> +		return;
> +
> +	/* Release both the cmd_rings mbufs */
> +	for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++)
> +		vmxnet3_rx_cmd_ring_release_mbufs(&rq->cmd_ring[i]);
> 
>  	ring0 = &rq->cmd_ring[0];
>  	ring1 = &rq->cmd_ring[1];
> @@ -287,8 +286,8 @@
> 
>  	size = sizeof(struct Vmxnet3_RxDesc) * (ring0->size + ring1->size);
>  	size += sizeof(struct Vmxnet3_RxCompDesc) * comp_ring->size;
> -	if (VMXNET3_VERSION_GE_3(hw) && rq->data_desc_size)
> -		size += rq->data_desc_size * data_ring->size;
> +	if (VMXNET3_VERSION_GE_3(rq->hw) && rq->data_desc_size)
> +		size += rq->data_desc_size * rq->data_ring.size;
> 
>  	memset(ring0->base, 0, size);
>  }
> --
> 1.9.1

I am sorry, please ignore this mail.
  

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index d9cf437..4fcceb4 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -259,17 +259,16 @@ 
 {
 	int i;
 	vmxnet3_rx_queue_t *rq = rxq;
-	struct vmxnet3_hw *hw = rq->hw;
 	struct vmxnet3_cmd_ring *ring0, *ring1;
 	struct vmxnet3_comp_ring *comp_ring;
-	struct vmxnet3_rx_data_ring *data_ring = &rq->data_ring;
 	int size;
 
-	if (rq != NULL) {
-		/* Release both the cmd_rings mbufs */
-		for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++)
-			vmxnet3_rx_cmd_ring_release_mbufs(&rq->cmd_ring[i]);
-	}
+	if (rq == NULL)
+		return;
+
+	/* Release both the cmd_rings mbufs */
+	for (i = 0; i < VMXNET3_RX_CMDRING_SIZE; i++)
+		vmxnet3_rx_cmd_ring_release_mbufs(&rq->cmd_ring[i]);
 
 	ring0 = &rq->cmd_ring[0];
 	ring1 = &rq->cmd_ring[1];
@@ -287,8 +286,8 @@ 
 
 	size = sizeof(struct Vmxnet3_RxDesc) * (ring0->size + ring1->size);
 	size += sizeof(struct Vmxnet3_RxCompDesc) * comp_ring->size;
-	if (VMXNET3_VERSION_GE_3(hw) && rq->data_desc_size)
-		size += rq->data_desc_size * data_ring->size;
+	if (VMXNET3_VERSION_GE_3(rq->hw) && rq->data_desc_size)
+		size += rq->data_desc_size * rq->data_ring.size;
 
 	memset(ring0->base, 0, size);
 }