[dpdk-dev,v2] enic: fix seg fault when releasing queues

  If device configuration failed due to a lack of resources, like if
there were more queues requested than available, the queue release
function is called with NULL pointers which were being dereferenced.

Skip releasing queues if they are NULL pointers. Also, if configuration
fails due to lack of resources, be more specific about which resources
are lacking.

Fixes: fefed3d1e62c ("enic: new driver")
Signed-off-by: John Daley <johndale@cisco.com>
---
v2: Log an error for all resource deficiencies not just the first one
found.

 drivers/net/enic/enic_main.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)
  

On Wed, May 25, 2016 at 07:45:00PM -0700, John Daley wrote:
> If device configuration failed due to a lack of resources, like if
> there were more queues requested than available, the queue release
> function is called with NULL pointers which were being dereferenced.
> 
> Skip releasing queues if they are NULL pointers. Also, if configuration
> fails due to lack of resources, be more specific about which resources
> are lacking.

The "also", and a a review of the code, indicates that the error message
changes should be a separate patch, as it's not related to the NULL check fix.
:-)


> 
> Fixes: fefed3d1e62c ("enic: new driver")
> Signed-off-by: John Daley <johndale@cisco.com>
> ---
> v2: Log an error for all resource deficiencies not just the first one
> found.
> 
>  drivers/net/enic/enic_main.c | 37 +++++++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 996f999..411d23c 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -426,14 +426,16 @@ int enic_alloc_intr_resources(struct enic *enic)
>  
>  void enic_free_rq(void *rxq)
>  {
> -	struct vnic_rq *rq = (struct vnic_rq *)rxq;
> -	struct enic *enic = vnic_dev_priv(rq->vdev);
> +	if (rxq != NULL) {
> +		struct vnic_rq *rq = (struct vnic_rq *)rxq;
> +		struct enic *enic = vnic_dev_priv(rq->vdev);
>  
> -	enic_rxmbuf_queue_release(enic, rq);
> -	rte_free(rq->mbuf_ring);
> -	rq->mbuf_ring = NULL;
> -	vnic_rq_free(rq);
> -	vnic_cq_free(&enic->cq[rq->index]);
> +		enic_rxmbuf_queue_release(enic, rq);
> +		rte_free(rq->mbuf_ring);
> +		rq->mbuf_ring = NULL;
> +		vnic_rq_free(rq);
> +		vnic_cq_free(&enic->cq[rq->index]);
> +	}
>  }

Is it not just easier to put a check at the top for: 
	if (rq == NULL)
		return;

Rather than putting the whole body of the function inside an if statement? It
would certainly be a smaller diff.

/Bruce
  

Patch broken into separate patches for fixing seg fault and improving
log messages.

John Daley (2):
  enic: fix seg fault when releasing queues
  enic: more specific out of resources error messages

 drivers/net/enic/enic_main.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)