RE: [net-next 05/13] i40e: rework exit flow of i40e_add_fdir_ethtool

[Keller, Jacob E]

> -Original Message-
> From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
> Sent: Tuesday, March 21, 2017 3:11 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com>; da...@davemloft.net
> Cc: Keller, Jacob E <jacob.e.kel...@intel.com>; netdev@vger.kernel.org;
> nhor...@redhat.com; sassm...@redhat.com; jogre...@redhat.com
> Subject: Re: [net-next 05/13] i40e: rework exit flow of i40e_add_fdir_ethtool
> 
> Hello!
> 
> On 3/21/2017 2:47 AM, Jeff Kirsher wrote:
> 
> > From: Jacob Keller <jacob.e.kel...@intel.com>
> >
> > Refactor the exit flow of the i40e_add_fdir_ethtool function. Move the
> > input_label to the end of the function, removing the dependency on
> 
> I don't see 'input_label' anywhere. Perhaps 'free_input' label was meant?
> 

You're correct. 

Thanks,
Jake

Re: [net-next 05/13] i40e: rework exit flow of i40e_add_fdir_ethtool

[Sergei Shtylyov]

Hello!

On 3/21/2017 2:47 AM, Jeff Kirsher wrote:


From: Jacob Keller 

Refactor the exit flow of the i40e_add_fdir_ethtool function. Move the
input_label to the end of the function, removing the dependency on


   I don't see 'input_label' anywhere. Perhaps 'free_input' label was meant?


having a non-zero return value. Add a comment explaining why it is ok
not to free the fdir data structure, because the structure is now stored
in the fdir_filter_list.

Change-Id: I723342181d59cd0c9f3b31140c37961ba37bb242
Signed-off-by: Jacob Keller 
Tested-by: Andrew Bowers 
Signed-off-by: Jeff Kirsher 
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 7a22b473dbdd..d16a5a6b24fc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2828,12 +2828,19 @@ static int i40e_add_fdir_ethtool(struct i40e_vsi *vsi,
}

ret = i40e_add_del_fdir(vsi, input, true);
-free_input:
if (ret)
-   kfree(input);
-   else
-   i40e_update_ethtool_fdir_entry(vsi, input, fsp->location, NULL);
+   goto free_input;
+
+   /* Add the input filter to the fdir_input_list, possibly replacing
+* a previous filter. Do not free the input structure after adding it
+* to the list as this would cause a use-after-free bug.
+*/
+   i40e_update_ethtool_fdir_entry(vsi, input, fsp->location, NULL);

+   return 0;
+
+free_input:
+   kfree(input);
return ret;
 }


MBR, Sergei