1. Patchew
  2. linux
  3. View series

[PATCH net V2] net: pds_core: Fix possible double free in error handling path

hyper posted 1 patch 2 years, 4 months ago
There is a newer version of this series
drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
[PATCH net V2] net: pds_core: Fix possible double free in error handling path
Posted by hyper 2 years, 4 months ago 
When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
calls kfree(padev) to free memory. We shouldn't call kfree(padev)
again in the error handling path.

Fix this by cleaning up the redundant kfree() and putting
the error handling back to where the errors happened.

Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
Signed-off-by: hyper <hyperlyzcs@gmail.com>
---
 drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
index 11c23a7f3172..fd1a5149c003 100644
--- a/drivers/net/ethernet/amd/pds_core/auxbus.c
+++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
@@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
 	if (err < 0) {
 		dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
 			 name, ERR_PTR(err));
-		goto err_out;
+		kfree(padev);
+		return ERR_PTR(err);
 	}
 
 	err = auxiliary_device_add(aux_dev);
 	if (err) {
 		dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
 			 name, ERR_PTR(err));
-		goto err_out_uninit;
+		auxiliary_device_uninit(aux_dev);
+		return ERR_PTR(err);
 	}
 
 	return padev;
-
-err_out_uninit:
-	auxiliary_device_uninit(aux_dev);
-err_out:
-	kfree(padev);
-	return ERR_PTR(err);
 }
 
 int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
-- 
2.36.1
Re: [PATCH net V2] net: pds_core: Fix possible double free in error handling path
Posted by Paolo Abeni 2 years, 4 months ago 
On Sun, 2024-03-03 at 16:49 +0800, hyper wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: hyper <hyperlyzcs@gmail.com>

Note that submitters are required to use real identity:

https://elixir.bootlin.com/linux/v6.8-rc7/source/Documentation/process/submitting-patches.rst#L438

Could you please repost avoiding the nick name?

You can retain the already collected acks.

Thanks,

Paolo
[PATCH net V3] net: pds_core: Fix possible double free in error handling path
Posted by Yongzhi Liu 2 years, 4 months ago 
When auxiliary_device_add() returns error and then calls
auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
calls kfree(padev) to free memory. We shouldn't call kfree(padev)
again in the error handling path.

Fix this by cleaning up the redundant kfree() and putting
the error handling back to where the errors happened.

Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com>
---
 drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
index 11c23a7f3172..fd1a5149c003 100644
--- a/drivers/net/ethernet/amd/pds_core/auxbus.c
+++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
@@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
 	if (err < 0) {
 		dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
 			 name, ERR_PTR(err));
-		goto err_out;
+		kfree(padev);
+		return ERR_PTR(err);
 	}
 
 	err = auxiliary_device_add(aux_dev);
 	if (err) {
 		dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
 			 name, ERR_PTR(err));
-		goto err_out_uninit;
+		auxiliary_device_uninit(aux_dev);
+		return ERR_PTR(err);
 	}
 
 	return padev;
-
-err_out_uninit:
-	auxiliary_device_uninit(aux_dev);
-err_out:
-	kfree(padev);
-	return ERR_PTR(err);
 }
 
 int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
-- 
2.36.1
Re: [PATCH net V3] net: pds_core: Fix possible double free in error handling path
Posted by Nelson, Shannon 2 years, 4 months ago 
On 3/6/2024 2:57 AM, Yongzhi Liu wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com>


Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>

> ---
>   drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 11c23a7f3172..fd1a5149c003 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>          if (err < 0) {
>                  dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
>                           name, ERR_PTR(err));
> -               goto err_out;
> +               kfree(padev);
> +               return ERR_PTR(err);
>          }
> 
>          err = auxiliary_device_add(aux_dev);
>          if (err) {
>                  dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
>                           name, ERR_PTR(err));
> -               goto err_out_uninit;
> +               auxiliary_device_uninit(aux_dev);
> +               return ERR_PTR(err);
>          }
> 
>          return padev;
> -
> -err_out_uninit:
> -       auxiliary_device_uninit(aux_dev);
> -err_out:
> -       kfree(padev);
> -       return ERR_PTR(err);
>   }
> 
>   int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> --
> 2.36.1
>
Re: [PATCH net V3] net: pds_core: Fix possible double free in error handling path
Posted by Wojciech Drewek 2 years, 4 months ago 

On 06.03.2024 11:57, Yongzhi Liu wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: Yongzhi Liu <hyperlyzcs@gmail.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 11c23a7f3172..fd1a5149c003 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>  	if (err < 0) {
>  		dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
>  			 name, ERR_PTR(err));
> -		goto err_out;
> +		kfree(padev);
> +		return ERR_PTR(err);
>  	}
>  
>  	err = auxiliary_device_add(aux_dev);
>  	if (err) {
>  		dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
>  			 name, ERR_PTR(err));
> -		goto err_out_uninit;
> +		auxiliary_device_uninit(aux_dev);
> +		return ERR_PTR(err);
>  	}
>  
>  	return padev;
> -
> -err_out_uninit:
> -	auxiliary_device_uninit(aux_dev);
> -err_out:
> -	kfree(padev);
> -	return ERR_PTR(err);
>  }
>  
>  int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
Re: [PATCH net V2] net: pds_core: Fix possible double free in error handling path
Posted by Breno Leitao 2 years, 4 months ago 
On Sun, Mar 03, 2024 at 04:49:54PM +0800, hyper wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: hyper <hyperlyzcs@gmail.com>

I liked this v2 better.

Reviewed-by: Breno Leitao <leitao@debian.org>
Re: [PATCH net V2] net: pds_core: Fix possible double free in error handling path
Posted by Nelson, Shannon 2 years, 4 months ago 
On 3/3/2024 12:49 AM, hyper wrote:
> 
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), Callback function pdsc_auxbus_dev_release
> calls kfree(padev) to free memory. We shouldn't call kfree(padev)
> again in the error handling path.
> 
> Fix this by cleaning up the redundant kfree() and putting
> the error handling back to where the errors happened.
> 
> Fixes: 4569cce43bc6 ("pds_core: add auxiliary_bus devices")
> Signed-off-by: hyper <hyperlyzcs@gmail.com>

Thanks.

Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>


> ---
>   drivers/net/ethernet/amd/pds_core/auxbus.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 11c23a7f3172..fd1a5149c003 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -160,23 +160,19 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>          if (err < 0) {
>                  dev_warn(cf->dev, "auxiliary_device_init of %s failed: %pe\n",
>                           name, ERR_PTR(err));
> -               goto err_out;
> +               kfree(padev);
> +               return ERR_PTR(err);
>          }
> 
>          err = auxiliary_device_add(aux_dev);
>          if (err) {
>                  dev_warn(cf->dev, "auxiliary_device_add of %s failed: %pe\n",
>                           name, ERR_PTR(err));
> -               goto err_out_uninit;
> +               auxiliary_device_uninit(aux_dev);
> +               return ERR_PTR(err);
>          }
> 
>          return padev;
> -
> -err_out_uninit:
> -       auxiliary_device_uninit(aux_dev);
> -err_out:
> -       kfree(padev);
> -       return ERR_PTR(err);
>   }
> 
>   int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> --
> 2.36.1
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.