Re: [PATCH] intel/i40e: Fix potential memory leak in i40e_init_recovery_mode()

[Jiri Pirko <jiri@resnulli.us>]

Tue, Dec 06, 2022 at 10:26:13AM CET, yuancan@huawei.com wrote:
>If i40e_vsi_mem_alloc() failed in i40e_init_recovery_mode(), the pf will be
>freed with the pf->vsi leaked.
>Fix by free pf->vsi in the error handling path.
>
>Fixes: 4ff0ee1af016 ("i40e: Introduce recovery mode support")
>Signed-off-by: Yuan Can <yuancan@huawei.com>
>---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>index b5dcd15ced36..d23081c224d6 100644
>--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>@@ -15536,6 +15536,7 @@ static int i40e_init_recovery_mode(struct i40e_pf *pf, struct i40e_hw *hw)
> 	pci_disable_pcie_error_reporting(pf->pdev);
> 	pci_release_mem_regions(pf->pdev);
> 	pci_disable_device(pf->pdev);
>+	kfree(pf->vsi);

This is not the only thing which is wrong on this error path. Just
quickly looking at the code:
- kfree(pf->qp_pile); should be called here as well
- if i40e_setup_misc_vector_for_recovery_mode() fails,
  unregister_netdev() needs to be called.
- if register_netdev() fails, netdev needs to be freed at least.
Basically the whole error path is completely wrong.
I suggest to:

1) rely on error path of i40e_probe() when call of
   i40e_init_recovery_mode() fails and don't do the cleanup of
   previously inited things in i40e_probe() locally.
2) implement a proper local error path in i40e_init_recovery_mode()


> 	kfree(pf);
> 
> 	return err;
>-- 
>2.17.1
>