RE: [PATCH] net: mana: Fix possible double free in error handling path

[Sai Krishna Gajula]

> -----Original Message-----
> From: Ma Ke <make24@xxxxxxxxxxx>
> Sent: Monday, June 24, 2024 8:51 AM
> To: kys@xxxxxxxxxxxxx; haiyangz@xxxxxxxxxxxxx; wei.liu@xxxxxxxxxx;
> decui@xxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx;
> kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; shradhagupta@xxxxxxxxxxxxxxxxxxx;
> horms@xxxxxxxxxx; kotaranov@xxxxxxxxxxxxx; linyunsheng@xxxxxxxxxx;
> schakrabarti@xxxxxxxxxxxxxxxxxxx; make24@xxxxxxxxxxx;
> erick.archer@xxxxxxxxxxx
> Cc: linux-hyperv@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH] net: mana: Fix possible double free in error
> handling path
> 
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev)
> to free memory. We shouldn't call kfree(padev) again in the error handling
> path. Signed-off-by: Ma Ke <make24@ iscas. ac. cn>

> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev)
> to free memory. We shouldn't call kfree(padev) again in the error handling
> path.
> 
> Signed-off-by: Ma Ke <make24@xxxxxxxxxxx>
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 31 +++++++++----------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..1754c92a6c15 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2785,8 +2785,10 @@ static int add_adev(struct gdma_dev *gd)
> 
>  	adev = &madev->adev;
>  	ret = mana_adev_idx_alloc();
> -	if (ret < 0)
> -		goto idx_fail;
> +	if (ret < 0) {
> +		kfree(madev);
> +		return ret;
> +	}
>  	adev->id = ret;
> 
>  	adev->name = "rdma";
> @@ -2795,26 +2797,21 @@ static int add_adev(struct gdma_dev *gd)
>  	madev->mdev = gd;
> 
>  	ret = auxiliary_device_init(adev);
> -	if (ret)
> -		goto init_fail;
> +	if (ret) {
> +		mana_adev_idx_free(adev->id);
> +		kfree(madev);
> +		return ret;
> +	}
> 
>  	ret = auxiliary_device_add(adev);
> -	if (ret)
> -		goto add_fail;
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		mana_adev_idx_free(adev->id);
> +		return ret;
> +	}
> 
>  	gd->adev = adev;
>  	return 0;
> -
> -add_fail:
> -	auxiliary_device_uninit(adev);
> -
> -init_fail:
> -	mana_adev_idx_free(adev->id);
> -
> -idx_fail:
> -	kfree(madev);
I think you can just avoid using add_fail and keep/retain rest of init_fail, idx_fail conditions in old way right?
> -
> -	return ret;
>  }
> 
>  int mana_probe(struct gdma_dev *gd, bool resuming)
> --
> 2.25.1
>