Thread (8 messages) 8 messages, 2 authors, 2017-10-02

Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null check

From: Jastrzebski, MichalX K <hidden>
Date: 2017-09-25 09:27:25

-----Original Message-----
From: Yigit, Ferruh
Sent: Friday, September 22, 2017 6:39 PM
To: Jastrzebski, MichalX K <redacted>;
skhare@vmware.com
Cc: dev@dpdk.org; Jain, Deepak K <redacted>; Kulasek,
TomaszX [off-list ref]; yongwang@vmware.com;
stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null
check

On 9/22/2017 1:39 PM, Michal Jastrzebski wrote:
quoted
From: Tomasz Kulasek <redacted>

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 <redacted>
---
 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
quoted
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) {
vmxnet3_dev_rx_queue_reset() is static function and only called from
single function [1], which already checks if the parameter is NULL.

What do you think just removing this check and keep rest same?

[1]
vmxnet3_dev_clear_queues()
Hi Ferruh,
Ok, we can try to do this as You suggest - we will see 
what coverity tells about that in the next build.
As long as vmxnet3_dev_clear_queues() checks if the parameter is NULL,
We can also classify this issue as False/Positive 
if our solution in this iteration doesn't help.

Michal.
quoted
-		/* 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);
 }
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help