> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, September 22, 2017 6:39 PM
> To: Jastrzebski, MichalX K <michalx.k.jastrzeb...@intel.com>;
> skh...@vmware.com
> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.j...@intel.com>; Kulasek,
> TomaszX <tomaszx.kula...@intel.com>; yongw...@vmware.com;
> sta...@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.kula...@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: yongw...@vmware.com
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Tomasz Kulasek <tomaszx.kula...@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);
> > }
> >
