Re: [PATCH 4.19 47/48] i40e: Memory leak in i40e_config_iwarp_qvlist

[Jesse Brandeburg]

On Tue, 11 Aug 2020 14:46:14 +0200
Pavel Machek <pavel@xxxxxxx> wrote:

> > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > @@ -449,16 +450,19 @@ static int i40e_config_iwarp_qvlist(stru
> >  			 "Incorrect number of iwarp vectors %u.
> > Maximum %u allowed.\n", qvlist_info->num_vectors,
> >  			 msix_vf);
> > -		goto err;
> > +		ret = -EINVAL;
> > +		goto err_out;
> >  	}
> 
> And it is no longer freeing data qvlist_info() in this path. Is that
> correct? Should it goto err_free instead? 

Hi Pavel, thanks for the review.

I believe it is still correct, the logic is a bit convoluted, but
tracing back, I see that the caller in i40e_main.c allocates a buffer,
calls this function (eventually) with that memory cast to the *input*
variable qvlist_info, and then the top caller frees the original buffer.

One thing that I'll admit is confusing here is that the *input* struct
qvlist_info is different than the vf->qvlist_info struct managed by
this function. Maybe that they have the same name was confusing to you?
It confused me for a moment while I investigated, but I believe there
is no actual problem.

The reason for the function working this way is that the input data is
from the VF message, and the driver data structures in the PF (i40e)
driver representing state of the VF are managed separately. 

Jesse