[PATCH] drm/amdgpu: fix ACP MFD device leak on init failure

Guangshuo Li posted 1 patch 3 days, 8 hours ago
drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
[PATCH] drm/amdgpu: fix ACP MFD device leak on init failure
Posted by Guangshuo Li 3 days, 8 hours ago
acp_hw_init() registers ACP child devices with mfd_add_devices() before
attaching them to the ACP power domain and initializing the hardware.

If attaching a child to the power domain fails, or if the ACP reset or
clock enable operation times out, the failure path frees only the
source cell, resource, and platform-data allocations. The child
platform devices already registered by mfd_add_devices() remain
registered and are never released.

Remove the children from the power domain and unregister the MFD
devices on failures that occur after mfd_add_devices() succeeds. Keep
mfd_add_devices() failures on the existing cleanup path since the MFD
core already rolls back partially registered children itself.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fixes: 25030321ba28 ("drm/amd: add pm domain for ACP IP sub blocks")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index 9014678d75ab..be4d00ac96b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -318,7 +318,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
 					  acp_genpd_add_device);
 		if (r)
-			goto failure;
+			goto failure_remove_mfd;
 		break;
 	}
 	default:
@@ -443,7 +443,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
 					  acp_genpd_add_device);
 		if (r)
-			goto failure;
+			goto failure_remove_mfd;
 	}
 
 	/* Assert Soft reset of ACP */
@@ -461,7 +461,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
 			r = -ETIMEDOUT;
-			goto failure;
+			goto failure_remove_mfd;
 		}
 		udelay(100);
 	}
@@ -479,7 +479,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
 			r = -ETIMEDOUT;
-			goto failure;
+			goto failure_remove_mfd;
 		}
 		udelay(100);
 	}
@@ -489,6 +489,11 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
 	cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
 	return 0;
 
+failure_remove_mfd:
+	device_for_each_child(adev->acp.parent, NULL,
+			      acp_genpd_remove_device);
+	mfd_remove_devices(adev->acp.parent);
+
 failure:
 	kfree(adev->acp.i2s_pdata);
 	kfree(adev->acp.acp_res);
-- 
2.43.0
Re: [PATCH] drm/amdgpu: fix ACP MFD device leak on init failure
Posted by Alex Deucher 2 days, 20 hours ago
Applied.  Thanks!

On Mon, Sep 21, 2026 at 6:51 AM Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> acp_hw_init() registers ACP child devices with mfd_add_devices() before
> attaching them to the ACP power domain and initializing the hardware.
>
> If attaching a child to the power domain fails, or if the ACP reset or
> clock enable operation times out, the failure path frees only the
> source cell, resource, and platform-data allocations. The child
> platform devices already registered by mfd_add_devices() remain
> registered and are never released.
>
> Remove the children from the power domain and unregister the MFD
> devices on failures that occur after mfd_add_devices() succeeds. Keep
> mfd_add_devices() failures on the existing cleanup path since the MFD
> core already rolls back partially registered children itself.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
>
> Fixes: 25030321ba28 ("drm/amd: add pm domain for ACP IP sub blocks")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> index 9014678d75ab..be4d00ac96b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
> @@ -318,7 +318,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
>                                           acp_genpd_add_device);
>                 if (r)
> -                       goto failure;
> +                       goto failure_remove_mfd;
>                 break;
>         }
>         default:
> @@ -443,7 +443,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 r = device_for_each_child(adev->acp.parent, &adev->acp.acp_genpd->gpd,
>                                           acp_genpd_add_device);
>                 if (r)
> -                       goto failure;
> +                       goto failure_remove_mfd;
>         }
>
>         /* Assert Soft reset of ACP */
> @@ -461,7 +461,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 if (--count == 0) {
>                         dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
>                         r = -ETIMEDOUT;
> -                       goto failure;
> +                       goto failure_remove_mfd;
>                 }
>                 udelay(100);
>         }
> @@ -479,7 +479,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>                 if (--count == 0) {
>                         dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
>                         r = -ETIMEDOUT;
> -                       goto failure;
> +                       goto failure_remove_mfd;
>                 }
>                 udelay(100);
>         }
> @@ -489,6 +489,11 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block)
>         cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
>         return 0;
>
> +failure_remove_mfd:
> +       device_for_each_child(adev->acp.parent, NULL,
> +                             acp_genpd_remove_device);
> +       mfd_remove_devices(adev->acp.parent);
> +
>  failure:
>         kfree(adev->acp.i2s_pdata);
>         kfree(adev->acp.acp_res);
> --
> 2.43.0
>