[PATCH 1/2] drm/amd/display: fix a potential dmub_srv memory leak

[PATCH 1/2] drm/amd/display: fix a potential dmub_srv memory leak

[Lang Yu]

Memory is allocated for dmub_srv in dm_sw_init(),
but not freed in dm_sw_fini(). Free it!

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e65214db904c..8f6766542c73 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2023,6 +2023,7 @@ static int dm_sw_fini(void *handle)
 
 	if (adev->dm.dmub_srv) {
 		dmub_srv_destroy(adev->dm.dmub_srv);
+		kfree(adev->dm.dmub_srv);
 		adev->dm.dmub_srv = NULL;
 	}
 
-- 
2.25.1

[PATCH 2/2] drm/amd/display: refine error handling for dm_dmub_sw_init()

[Lang Yu]

To avoid potential memory leaks, refine error handling for
dm_dmub_sw_init().

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 37 ++++++++++++++-----
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8f6766542c73..ef6800dc0215 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1890,19 +1890,19 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 
 	default:
 		/* ASIC doesn't support DMUB. */
-		return 0;
+		return -EINVAL;
 	}
 
 	r = request_firmware_direct(&adev->dm.dmub_fw, fw_name_dmub, adev->dev);
 	if (r) {
 		DRM_ERROR("DMUB firmware loading failed: %d\n", r);
-		return 0;
+		return r;
 	}
 
 	r = amdgpu_ucode_validate(adev->dm.dmub_fw);
 	if (r) {
 		DRM_ERROR("Couldn't validate DMUB firmware: %d\n", r);
-		return 0;
+		goto release_firmware;
 	}
 
 	hdr = (const struct dmcub_firmware_header_v1_0 *)adev->dm.dmub_fw->data;
@@ -1926,7 +1926,8 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 
 	if (!dmub_srv) {
 		DRM_ERROR("Failed to allocate DMUB service!\n");
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto release_firmware;
 	}
 
 	memset(&create_params, 0, sizeof(create_params));
@@ -1939,7 +1940,8 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 	status = dmub_srv_create(dmub_srv, &create_params);
 	if (status != DMUB_STATUS_OK) {
 		DRM_ERROR("Error creating DMUB service: %d\n", status);
-		return -EINVAL;
+		r = -EINVAL;
+		goto free_dmub_srv;
 	}
 
 	/* Calculate the size of all the regions for the DMUB service. */
@@ -1963,7 +1965,8 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 
 	if (status != DMUB_STATUS_OK) {
 		DRM_ERROR("Error calculating DMUB region info: %d\n", status);
-		return -EINVAL;
+		r = -EINVAL;
+		goto free_dmub_srv;
 	}
 
 	/*
@@ -1975,8 +1978,7 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 				    &adev->dm.dmub_bo_gpu_addr,
 				    &adev->dm.dmub_bo_cpu_addr);
 	if (r)
-		return r;
-
+		goto free_dmub_srv;
 	/* Rebase the regions on the framebuffer address. */
 	memset(&fb_params, 0, sizeof(fb_params));
 	fb_params.cpu_addr = adev->dm.dmub_bo_cpu_addr;
@@ -1990,16 +1992,31 @@ static int dm_dmub_sw_init(struct amdgpu_device *adev)
 	if (!fb_info) {
 		DRM_ERROR(
 			"Failed to allocate framebuffer info for DMUB service!\n");
-		return -ENOMEM;
+		r =  -ENOMEM;
+		goto free_dmub_bo;
 	}
 
 	status = dmub_srv_calc_fb_info(dmub_srv, &fb_params, fb_info);
 	if (status != DMUB_STATUS_OK) {
 		DRM_ERROR("Error calculating DMUB FB info: %d\n", status);
-		return -EINVAL;
+		r = -EINVAL;
+		goto free_dmub_bo;
 	}
 
 	return 0;
+
+free_dmub_bo:
+	amdgpu_bo_free_kernel(&adev->dm.dmub_bo,
+			      &adev->dm.dmub_bo_gpu_addr,
+			      &adev->dm.dmub_bo_cpu_addr);
+free_dmub_srv:
+	kfree(adev->dm.dmub_srv);
+	adev->dm.dmub_srv = NULL;
+release_firmware:
+	release_firmware(adev->dm.dmub_fw);
+	adev->dm.dmub_fw = NULL;
+
+	return r;
 }
 
 static int dm_sw_init(void *handle)
-- 
2.25.1