RE: [PATCH] net: mana: Fix possible double free in error handling path
From: Sai Krishna Gajula
Date: Mon Jun 24 2024 - 04:39:08 EST
> -----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
>