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

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

[Ma Ke]

When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), callback function adev_release
calls kfree(madev). We shouldn't call kfree(madev) again
in the error handling path. Set 'madev' to NULL.

Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v2:
- streamlined the patch according suggestions;
- revised the description.
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index d087cf954f75..608ad31a9702 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
 	if (ret)
 		goto init_fail;
 
+	/* madev is owned by the auxiliary device */
+	madev = NULL;
 	ret = auxiliary_device_add(adev);
 	if (ret)
 		goto add_fail;
-- 
2.25.1

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

[Przemek Kitszel]

On 6/25/24 10:38, Ma Ke wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
> calls kfree(madev). We shouldn't call kfree(madev) again
> in the error handling path. Set 'madev' to NULL.
> 
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.
> ---
>   drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..608ad31a9702 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
>   	if (ret)
>   		goto init_fail;
>   
> +	/* madev is owned by the auxiliary device */
> +	madev = NULL;
>   	ret = auxiliary_device_add(adev);
>   	if (ret)
>   		goto add_fail;

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

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

[Konstantin Taranov]

> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev).
> We shouldn't call kfree(madev) again in the error handling path. Set 'madev'
> to NULL.
> 
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.
> ---
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d087cf954f75..608ad31a9702 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -2798,6 +2798,8 @@ static int add_adev(struct gdma_dev *gd)
>         if (ret)
>                 goto init_fail;
> 
> +       /* madev is owned by the auxiliary device */
> +       madev = NULL;
>         ret = auxiliary_device_add(adev);
>         if (ret)
>                 goto add_fail;
> --
> 2.25.1

Reviewed-by: Konstantin Taranov <kotaranov@microsoft.com>

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

[Konstantin Taranov]

> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release calls kfree(madev).
> We shouldn't call kfree(madev) again in the error handling path. Set 'madev'
> to NULL.
> 
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
> ---
> Changes in v2:
> - streamlined the patch according suggestions;
> - revised the description.

The change is ok, but the commit message is missing a "Fixes" tag/line.
- Konstantin