Vulnerability: ** DISPUTED ** 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.
Vulnerability ID: CVE-2019-19067
CWE: CWE-401
Severity score: 6.9
Project: https://github.com/torvalds/linux
Published: 2019-11-18
File: drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c
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>
Commit author: Navid Emamdoost

Vulnerable version (before fix):
 static int acp_hw_init(void *handle) 	u32 val = 0; 	u32 count = 0; 	struct device *dev; 	struct i2s_platform_data *i2s_pdata;  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;  static int acp_hw_init(void *handle) 	adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell), 							GFP_KERNEL);  	if (adev->acp.acp_cell == NULL) 		return -ENOMEM;  	adev->acp.acp_res = kcalloc(5, sizeof(struct resource), GFP_KERNEL); 	if (adev->acp.acp_res == NULL) { 		kfree(adev->acp.acp_cell); 		return -ENOMEM; 	}  	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; 	}  	switch (adev->asic_type) { static int acp_hw_init(void *handle) 	r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 								ACP_DEVS); 	if (r) 		return r;  	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; 		} 	}  static int acp_hw_init(void *handle) 			break; 		if (--count == 0) { 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); 			return -ETIMEDOUT; 		} 		udelay(100); 	} static int acp_hw_init(void *handle) 			break; 		if (--count == 0) { 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); 			return -ETIMEDOUT; 		} 		udelay(100); 	} static int acp_hw_init(void *handle) 	val &= ~ACP_SOFT_RESET__SoftResetAud_MASK; 	cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val); 	return 0; }  /**

Fixed version (after fix):
 static int acp_hw_init(void *handle) 	u32 val = 0; 	u32 count = 0; 	struct device *dev; 	struct i2s_platform_data *i2s_pdata = NULL;  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;  static int acp_hw_init(void *handle) 	adev->acp.acp_cell = kcalloc(ACP_DEVS, sizeof(struct mfd_cell), 							GFP_KERNEL);  	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) { 		r = -ENOMEM; 		goto failure; 	}  	i2s_pdata = kcalloc(3, sizeof(struct i2s_platform_data), GFP_KERNEL); 	if (i2s_pdata == NULL) { 		r = -ENOMEM; 		goto failure; 	}  	switch (adev->asic_type) { static int acp_hw_init(void *handle) 	r = mfd_add_hotplug_devices(adev->acp.parent, adev->acp.acp_cell, 								ACP_DEVS); 	if (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"); 			goto failure; 		} 	}  static int acp_hw_init(void *handle) 			break; 		if (--count == 0) { 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); 			r = -ETIMEDOUT; 			goto failure; 		} 		udelay(100); 	} static int acp_hw_init(void *handle) 			break; 		if (--count == 0) { 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); 			r = -ETIMEDOUT; 			goto failure; 		} 		udelay(100); 	} static int acp_hw_init(void *handle) 	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; }  /**

Patch:
patch: @@ -189,7 +189,7 @@ static int acp_hw_init(void *handle)
 	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;
 
@@ -231,20 +231,21 @@ static int acp_hw_init(void *handle)
 	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) {
@@ -341,14 +342,14 @@ static int acp_hw_init(void *handle)
 	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;
 		}
 	}
 
@@ -367,7 +368,8 @@ static int acp_hw_init(void *handle)
 			break;
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
-			return -ETIMEDOUT;
+			r = -ETIMEDOUT;
+			goto failure;
 		}
 		udelay(100);
 	}
@@ -384,7 +386,8 @@ static int acp_hw_init(void *handle)
 			break;
 		if (--count == 0) {
 			dev_err(&adev->pdev->dev, "Failed to reset ACP\n");
-			return -ETIMEDOUT;
+			r = -ETIMEDOUT;
+			goto failure;
 		}
 		udelay(100);
 	}
@@ -393,6 +396,13 @@ static int acp_hw_init(void *handle)
 	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;
 }
 
 /**
path: drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c