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

Message ID 20170929130402.32196-1-michalx.k.jastrzebski@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Michal Jastrzebski Sept. 29, 2017, 1:04 p.m. UTC
  Coverity reports 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 removes NULL checking of "rq" from function
vmxnet3_dev_rx_queue_reset as it is already checked against NULL
one level up the callstack (function vmxnet3_dev_clear_queues).

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>
Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
  

Comments

Michal Jastrzebski Oct. 2, 2017, 1:58 p.m. UTC | #1
> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Friday, September 29, 2017 3:04 PM
> To: skhare@vmware.com
> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.jain@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Jastrzebski, MichalX K
> <michalx.k.jastrzebski@intel.com>; yongwang@vmware.com;
> stable@dpdk.org; Kulasek, TomaszX <tomaszx.kulasek@intel.com>
> Subject: [PATCH v2] net/vmxnet3: fix dereference before null check
> 
> Coverity reports 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 removes NULL checking of "rq" from function
> vmxnet3_dev_rx_queue_reset as it is already checked against NULL
> one level up the callstack (function vmxnet3_dev_clear_queues).
> 
> 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>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> ---
>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index d9cf437..0f8cfff 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -265,11 +265,9 @@ vmxnet3_dev_rx_queue_reset(void *rxq)
>  	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]);
> -	}
> +	/* 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];
> --
> 2.7.4

Hi Shrikrishna Khare,
I would like to ask for a feedback regarding proposed fix.
If everything is ok with it, please send acked-by.

Best regards
Michal.
  
Ferruh Yigit Oct. 2, 2017, 9:39 p.m. UTC | #2
On 9/29/2017 2:04 PM, Michal Jastrzebski wrote:
> Coverity reports 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 removes NULL checking of "rq" from function
> vmxnet3_dev_rx_queue_reset as it is already checked against NULL
> one level up the callstack (function vmxnet3_dev_clear_queues).
> 
> 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>
> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
  
Ferruh Yigit Oct. 2, 2017, 9:45 p.m. UTC | #3
On 10/2/2017 10:39 PM, Ferruh Yigit wrote:
> On 9/29/2017 2:04 PM, Michal Jastrzebski wrote:
>> Coverity reports 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 removes NULL checking of "rq" from function
>> vmxnet3_dev_rx_queue_reset as it is already checked against NULL
>> one level up the callstack (function vmxnet3_dev_clear_queues).
>>
>> 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>
>> Signed-off-by: Michal Jastrzebski <michalx.k.jastrzebski@intel.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index d9cf437..0f8cfff 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -265,11 +265,9 @@  vmxnet3_dev_rx_queue_reset(void *rxq)
 	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]);
-	}
+	/* 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];