* [PATCH] net/vmxnet3: fix dereference before null check
@ 2017-09-22 12:39 Michal Jastrzebski
2017-09-22 16:39 ` [dpdk-stable] " Ferruh Yigit
2017-09-29 13:04 ` [PATCH v2] " Michal Jastrzebski
0 siblings, 2 replies; 10+ messages in thread
From: Michal Jastrzebski @ 2017-09-22 12:39 UTC (permalink / raw)
To: skhare; +Cc: dev, deepak.k.jain, Tomasz Kulasek, yongwang, stable
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null check
2017-09-22 12:39 [PATCH] net/vmxnet3: fix dereference before null check Michal Jastrzebski
@ 2017-09-22 16:39 ` Ferruh Yigit
2017-09-25 9:27 ` Jastrzebski, MichalX K
2017-09-29 13:04 ` [PATCH v2] " Michal Jastrzebski
1 sibling, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2017-09-22 16:39 UTC (permalink / raw)
To: Michal Jastrzebski, skhare
Cc: dev, deepak.k.jain, Tomasz Kulasek, yongwang, stable
On 9/22/2017 1:39 PM, Michal Jastrzebski wrote:
> 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) {
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()
> - /* 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);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null check
2017-09-22 16:39 ` [dpdk-stable] " Ferruh Yigit
@ 2017-09-25 9:27 ` Jastrzebski, MichalX K
2017-09-25 10:02 ` Ferruh Yigit
0 siblings, 1 reply; 10+ messages in thread
From: Jastrzebski, MichalX K @ 2017-09-25 9:27 UTC (permalink / raw)
To: Yigit, Ferruh, skhare
Cc: dev, Jain, Deepak K, Kulasek, TomaszX, yongwang, stable
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, September 22, 2017 6:39 PM
> To: Jastrzebski, MichalX K <michalx.k.jastrzebski@intel.com>;
> skhare@vmware.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: Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null
> check
>
> On 9/22/2017 1:39 PM, Michal Jastrzebski wrote:
> > 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) {
>
> 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.
> > - /* 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);
> > }
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null check
2017-09-25 9:27 ` Jastrzebski, MichalX K
@ 2017-09-25 10:02 ` Ferruh Yigit
0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2017-09-25 10:02 UTC (permalink / raw)
To: Jastrzebski, MichalX K, skhare
Cc: dev, Jain, Deepak K, Kulasek, TomaszX, yongwang, stable
On 9/25/2017 10:27 AM, Jastrzebski, MichalX K wrote:
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, September 22, 2017 6:39 PM
>> To: Jastrzebski, MichalX K <michalx.k.jastrzebski@intel.com>;
>> skhare@vmware.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: Re: [dpdk-stable] [PATCH] net/vmxnet3: fix dereference before null
>> check
>>
>> On 9/22/2017 1:39 PM, Michal Jastrzebski wrote:
>>> 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) {
>>
>> 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.
What I understand is, coverity complains about dereferencing variable
(rq) before NULL check, so removing NULL check should fix the warning.
> 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.
Agreed, thanks.
>
> Michal.
>
<...>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] net/vmxnet3: fix dereference before null check
2017-09-22 12:39 [PATCH] net/vmxnet3: fix dereference before null check Michal Jastrzebski
2017-09-22 16:39 ` [dpdk-stable] " Ferruh Yigit
@ 2017-09-29 13:04 ` Michal Jastrzebski
2017-10-02 13:58 ` Jastrzebski, MichalX K
2017-10-02 21:39 ` Ferruh Yigit
1 sibling, 2 replies; 10+ messages in thread
From: Michal Jastrzebski @ 2017-09-29 13:04 UTC (permalink / raw)
To: skhare
Cc: dev, deepak.k.jain, ferruh.yigit, Michal Jastrzebski, yongwang,
stable, Tomasz Kulasek
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] net/vmxnet3: fix dereference before null check
2017-09-29 13:04 ` [PATCH v2] " Michal Jastrzebski
@ 2017-10-02 13:58 ` Jastrzebski, MichalX K
2017-10-02 21:39 ` Ferruh Yigit
1 sibling, 0 replies; 10+ messages in thread
From: Jastrzebski, MichalX K @ 2017-10-02 13:58 UTC (permalink / raw)
To: skhare
Cc: dev, Jain, Deepak K, Yigit, Ferruh, yongwang, stable, Kulasek, TomaszX
> -----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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] net/vmxnet3: fix dereference before null check
2017-09-29 13:04 ` [PATCH v2] " Michal Jastrzebski
2017-10-02 13:58 ` Jastrzebski, MichalX K
@ 2017-10-02 21:39 ` Ferruh Yigit
2017-10-02 21:45 ` [dpdk-stable] " Ferruh Yigit
1 sibling, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2017-10-02 21:39 UTC (permalink / raw)
To: Michal Jastrzebski, skhare
Cc: dev, deepak.k.jain, yongwang, stable, Tomasz Kulasek
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [dpdk-stable] [PATCH v2] net/vmxnet3: fix dereference before null check
2017-10-02 21:39 ` Ferruh Yigit
@ 2017-10-02 21:45 ` Ferruh Yigit
0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2017-10-02 21:45 UTC (permalink / raw)
To: Michal Jastrzebski, skhare
Cc: dev, deepak.k.jain, yongwang, stable, Tomasz Kulasek
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net/vmxnet3: fix dereference before null check
2017-09-22 13:07 [PATCH] " Michal Jastrzebski
@ 2017-09-22 13:19 ` Jastrzebski, MichalX K
0 siblings, 0 replies; 10+ messages in thread
From: Jastrzebski, MichalX K @ 2017-09-22 13:19 UTC (permalink / raw)
To: Jastrzebski, MichalX K, yliu, maxime.coquelin
Cc: dev, Jain, Deepak K, Kulasek, TomaszX, yongwang, stable
> -----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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] net/vmxnet3: fix dereference before null check
@ 2017-09-22 13:07 Michal Jastrzebski
2017-09-22 13:19 ` Jastrzebski, MichalX K
0 siblings, 1 reply; 10+ messages in thread
From: Michal Jastrzebski @ 2017-09-22 13:07 UTC (permalink / raw)
To: yliu, maxime.coquelin
Cc: dev, deepak.k.jain, Tomasz Kulasek, yongwang, stable
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-02 21:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22 12:39 [PATCH] net/vmxnet3: fix dereference before null check Michal Jastrzebski
2017-09-22 16:39 ` [dpdk-stable] " Ferruh Yigit
2017-09-25 9:27 ` Jastrzebski, MichalX K
2017-09-25 10:02 ` Ferruh Yigit
2017-09-29 13:04 ` [PATCH v2] " Michal Jastrzebski
2017-10-02 13:58 ` Jastrzebski, MichalX K
2017-10-02 21:39 ` Ferruh Yigit
2017-10-02 21:45 ` [dpdk-stable] " Ferruh Yigit
2017-09-22 13:07 [PATCH] " Michal Jastrzebski
2017-09-22 13:19 ` Jastrzebski, MichalX K
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.