CVE-2019-19067: Missing Release of Memory after Effective Lifetime (CWE-401)
value: Four memory leaks in the acp_hw_init() function in drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c in the Linux kernel before 5.3.8 allow attackers to cause a denial of service (memory consumption) by triggering mfd_add_hotplug_devices() or pm_genpd_add_device() failures, aka CID-57be09c6e874. NOTE: third parties dispute the relevance of this because the attacker must already have privileges for module loading
CWE description: The product does not sufficiently track and release allocated memory after it has been used, which slowly consumes remaining memory.
CVSS v3 base score: 4.4
Published: 2019-11-18T06:15Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
Commit date: 2019-10-02 12:58:33 -0500
Commit message: drm/amdgpu: fix multiple memory leaks in acp_hw_init

In acp_hw_init there are some allocations that needs to be released in
case of failure:

1- adev->acp.acp_genpd should be released if any allocation attemp for
adev->acp.acp_cell, adev->acp.acp_res or i2s_pdata fails.
2- all of those allocations should be released if
mfd_add_hotplug_devices or pm_genpd_add_device fail.
3- Release is needed in case of time out values expire.

Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Diff: diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
index eba42c752bca..82155ac3288a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
@@ -187,11 +187,11 @@ static int acp_hw_init(void *handle)
 	int r, i;
 	uint64_t acp_base;
 	u32 val = 0;
 	u32 count = 0;
 	struct device *dev;
-	struct i2s_platform_data *i2s_pdata;
+	struct i2s_platform_data *i2s_pdata = NULL;
 
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
 	const struct amdgpu_ip_block *ip_block =
 		amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_ACP);
@@ -229,24 +229,25 @@ static int acp_hw_init(void *handle)
 	pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false);
 
 	adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell),
 							GFP_KERNEL);
 
-	if (adev->acp.acp_cell == NULL)
-		return -ENOMEM;
+	if (adev->acp.acp_cell == NULL) {
+		r = -ENOMEM;
+		goto failure;
+	}
 
 	adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL);
 	if (adev->acp.acp_res == NULL) {
-		kfree(adev->acp.acp_cell);
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto failure;
 	}
 
 	i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL);
 	if (i2s_pdata == NULL) {
-		kfree(adev->acp.acp_res);
-		kfree(adev->acp.acp_cell);
-		return -ENOMEM;
+		r = -ENOMEM;
+		goto failure;
 	}
 
 	switch (adev->asic_type) {
 	case CHIP_STONEY:
 		i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET |
@@ -339,18 +340,18 @@ static int acp_hw_init(void *handle)
 	adev->acp.acp_cell[3].pdata_size = sizeof(struct i2s_platform_data);
 
 	r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell,
 								ACP_DEVS);
 	if (r)
-		return r;
+		goto failure;
 
 	for (i = 0; i < ACP_DEVS ; i++) {
 		dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i);
 		r = pm_genpd_add_device(&adev->acp.acp_genpd->gpd, dev);
 		if (r) {
 			dev_err(dev, "Failed to add dev to genpd\n");
-			return r;
+			goto failure;
 		}
 	}
 
 
 	/* Assert Soft reset of ACP */
@@ -365,11 +366,12 @@ static int acp_hw_init(void *handle)
 		if (ACP_SOFT_RESET__SoftResetAudDone_MASK ==
 		    (val & ACP_SOFT_RESET__SoftResetAudDone_MASK))
 			break;
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
-			return -ETIMEDOUT;
+			r = -ETIMEDOUT;
+			goto failure;
 		}
 		udelay(100);
 	}
 	/* Enable clock to ACP and wait until the clock is enabled */
 	val = cgs_read_register(adev->acp.cgs_device, mmACP_CONTROL);
@@ -382,19 +384,27 @@ static int acp_hw_init(void *handle)
 		val = cgs_read_register(adev->acp.cgs_device, mmACP_STATUS);
 		if (val & (u32) 0x1)
 			break;
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
-			return -ETIMEDOUT;
+			r = -ETIMEDOUT;
+			goto failure;
 		}
 		udelay(100);
 	}
 	/* Deassert the SOFT RESET flags */
 	val = cgs_read_register(adev->acp.cgs_device, mmACP_SOFT_RESET);
 	val &= ~ACP_SOFT_RESET__SoftResetAud_MASK;
 	cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val);
 	return 0;
+
+failure:
+	kfree(i2s_pdata);
+	kfree(adev->acp.acp_res);
+	kfree(adev->acp.acp_cell);
+	kfree(adev->acp.acp_genpd);
+	return r;
 }
 
 /**
  * acp_hw_fini - stop the hardware block
  *

Vulnerable code: 	struct i2s_platform_data *i2s_pdata;
	if (adev->acp.acp_cell == NULL)
		return -ENOMEM;
		kfree(adev->acp.acp_cell);
		return -ENOMEM;
		kfree(adev->acp.acp_res);
		kfree(adev->acp.acp_cell);
		return -ENOMEM;
		return r;
			return r;
			return -ETIMEDOUT;
			return -ETIMEDOUT;
Fixed code: 	struct i2s_platform_data *i2s_pdata = NULL;
	if (adev->acp.acp_cell == NULL) {
		r = -ENOMEM;
		goto failure;
	}
		r = -ENOMEM;
		goto failure;
		r = -ENOMEM;
		goto failure;
		goto failure;
			goto failure;
			r = -ETIMEDOUT;
			goto failure;
			r = -ETIMEDOUT;
			goto failure;

failure:
	kfree(i2s_pdata);
	kfree(adev->acp.acp_res);
	kfree(adev->acp.acp_cell);
	kfree(adev->acp.acp_genpd);
	return r;