[PATCH v2 2/2] drm/amd/powerplay: Fix KASAN user after after free on driver unload.

Andrey Grodzovsky andrey.grodzovsky at amd.com
Thu Mar 15 16:15:11 UTC 2018


Reusing local handle to initialize BO without resetting it to
NULL is wrong since it causes amdgpu_bo_create_reserved to skip
new BO creation and just reuse the given pointer for pinning.

v2:
Get rid of mc_addr and kaddr.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
---
 drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c   | 21 +++-------
 .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   | 48 +++++++---------------
 2 files changed, 21 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
index e2ee23a..01fcfb9 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
@@ -327,10 +327,7 @@ static int rv_start_smu(struct pp_hwmgr *hwmgr)
 
 static int rv_smu_init(struct pp_hwmgr *hwmgr)
 {
-	struct amdgpu_bo *handle = NULL;
 	struct rv_smumgr *priv;
-	uint64_t mc_addr;
-	void *kaddr = NULL;
 	int r;
 
 	priv = kzalloc(sizeof(struct rv_smumgr), GFP_KERNEL);
@@ -345,9 +342,9 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
 			sizeof(Watermarks_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[WMTABLE].handle,
+			&priv->smu_tables.entry[WMTABLE].mc_addr,
+			&priv->smu_tables.entry[WMTABLE].table);
 
 	if (r)
 		return -EINVAL;
@@ -355,18 +352,15 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
 	priv->smu_tables.entry[WMTABLE].version = 0x01;
 	priv->smu_tables.entry[WMTABLE].size = sizeof(Watermarks_t);
 	priv->smu_tables.entry[WMTABLE].table_id = TABLE_WATERMARKS;
-	priv->smu_tables.entry[WMTABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[WMTABLE].table = kaddr;
-	priv->smu_tables.entry[WMTABLE].handle = handle;
 
 	/* allocate space for watermarks table */
 	r = amdgpu_bo_create_kernel((struct amdgpu_device *)hwmgr->adev,
 			sizeof(DpmClocks_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[CLOCKTABLE].handle,
+			&priv->smu_tables.entry[CLOCKTABLE].mc_addr,
+			&priv->smu_tables.entry[CLOCKTABLE].table);
 
 	if (r) {
 		amdgpu_bo_free_kernel(&priv->smu_tables.entry[WMTABLE].handle,
@@ -378,9 +372,6 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
 	priv->smu_tables.entry[CLOCKTABLE].version = 0x01;
 	priv->smu_tables.entry[CLOCKTABLE].size = sizeof(DpmClocks_t);
 	priv->smu_tables.entry[CLOCKTABLE].table_id = TABLE_DPMCLOCKS;
-	priv->smu_tables.entry[CLOCKTABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[CLOCKTABLE].table = kaddr;
-	priv->smu_tables.entry[CLOCKTABLE].handle = handle;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
index 15e1afa..c13cf4e 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
@@ -377,10 +377,7 @@ static int vega10_verify_smc_interface(struct pp_hwmgr *hwmgr)
 
 static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 {
-	struct amdgpu_bo *handle = NULL;
 	struct vega10_smumgr *priv;
-	uint64_t mc_addr;
-	void *kaddr = NULL;
 	unsigned long tools_size;
 	int ret;
 	struct cgs_firmware_info info = {0};
@@ -403,9 +400,9 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 			sizeof(PPTable_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[PPTABLE].handle,
+			&priv->smu_tables.entry[PPTABLE].mc_addr,
+			&priv->smu_tables.entry[PPTABLE].table);
 
 	if (ret)
 		return -EINVAL;
@@ -413,18 +410,15 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 	priv->smu_tables.entry[PPTABLE].version = 0x01;
 	priv->smu_tables.entry[PPTABLE].size = sizeof(PPTable_t);
 	priv->smu_tables.entry[PPTABLE].table_id = TABLE_PPTABLE;
-	priv->smu_tables.entry[PPTABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[PPTABLE].table = kaddr;
-	priv->smu_tables.entry[PPTABLE].handle = handle;
 
 	/* allocate space for watermarks table */
 	ret = amdgpu_bo_create_kernel((struct amdgpu_device *)hwmgr->adev,
 			sizeof(Watermarks_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[WMTABLE].handle,
+			&priv->smu_tables.entry[WMTABLE].mc_addr,
+			&priv->smu_tables.entry[WMTABLE].table);
 
 	if (ret)
 		goto err0;
@@ -432,18 +426,15 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 	priv->smu_tables.entry[WMTABLE].version = 0x01;
 	priv->smu_tables.entry[WMTABLE].size = sizeof(Watermarks_t);
 	priv->smu_tables.entry[WMTABLE].table_id = TABLE_WATERMARKS;
-	priv->smu_tables.entry[WMTABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[WMTABLE].table = kaddr;
-	priv->smu_tables.entry[WMTABLE].handle = handle;
 
 	/* allocate space for AVFS table */
 	ret = amdgpu_bo_create_kernel((struct amdgpu_device *)hwmgr->adev,
 			sizeof(AvfsTable_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[AVFSTABLE].handle,
+			&priv->smu_tables.entry[AVFSTABLE].mc_addr,
+			&priv->smu_tables.entry[AVFSTABLE].table);
 
 	if (ret)
 		goto err1;
@@ -451,9 +442,6 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 	priv->smu_tables.entry[AVFSTABLE].version = 0x01;
 	priv->smu_tables.entry[AVFSTABLE].size = sizeof(AvfsTable_t);
 	priv->smu_tables.entry[AVFSTABLE].table_id = TABLE_AVFS;
-	priv->smu_tables.entry[AVFSTABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[AVFSTABLE].table = kaddr;
-	priv->smu_tables.entry[AVFSTABLE].handle = handle;
 
 	tools_size = 0x19000;
 	if (tools_size) {
@@ -461,17 +449,14 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 				tools_size,
 				PAGE_SIZE,
 				AMDGPU_GEM_DOMAIN_VRAM,
-				&handle,
-				&mc_addr,
-				&kaddr);
+				&priv->smu_tables.entry[TOOLSTABLE].handle,
+				&priv->smu_tables.entry[TOOLSTABLE].mc_addr,
+				&priv->smu_tables.entry[TOOLSTABLE].table);
 		if (ret)
 			goto err2;
 		priv->smu_tables.entry[TOOLSTABLE].version = 0x01;
 		priv->smu_tables.entry[TOOLSTABLE].size = tools_size;
 		priv->smu_tables.entry[TOOLSTABLE].table_id = TABLE_PMSTATUSLOG;
-		priv->smu_tables.entry[TOOLSTABLE].mc_addr = mc_addr;
-		priv->smu_tables.entry[TOOLSTABLE].table = kaddr;
-		priv->smu_tables.entry[TOOLSTABLE].handle = handle;
 	}
 
 	/* allocate space for AVFS Fuse table */
@@ -479,18 +464,15 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
 			sizeof(AvfsFuseOverride_t),
 			PAGE_SIZE,
 			AMDGPU_GEM_DOMAIN_VRAM,
-			&handle,
-			&mc_addr,
-			&kaddr);
+			&priv->smu_tables.entry[AVFSFUSETABLE].handle,
+			&priv->smu_tables.entry[AVFSFUSETABLE].mc_addr,
+			&priv->smu_tables.entry[AVFSFUSETABLE].table);
 	if (ret)
 		goto err3;
 
 	priv->smu_tables.entry[AVFSFUSETABLE].version = 0x01;
 	priv->smu_tables.entry[AVFSFUSETABLE].size = sizeof(AvfsFuseOverride_t);
 	priv->smu_tables.entry[AVFSFUSETABLE].table_id = TABLE_AVFS_FUSE_OVERRIDE;
-	priv->smu_tables.entry[AVFSFUSETABLE].mc_addr = mc_addr;
-	priv->smu_tables.entry[AVFSFUSETABLE].table = kaddr;
-	priv->smu_tables.entry[AVFSFUSETABLE].handle = handle;
 
 	return 0;
 
-- 
2.7.4



More information about the amd-gfx mailing list