subject:"Re\: \[PATCH 2\/2\] drm\/amd\/powerplay\: Fix KASAN user after free on driver unload."

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

2018-03-15 Thread Zhu, Rex

Apply this patch with Christian's suggestions.


Thanks.


Best Regards

Rex



From: Christian König <ckoenig.leichtzumer...@gmail.com>
Sent: Thursday, March 15, 2018 2:21 AM
To: Grodzovsky, Andrey; amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex; Koenig, Christian
Subject: Re: [PATCH 2/2] drm/amd/powerplay: Fix KASAN user after free on driver 
unload.

Am 14.03.2018 um 19:07 schrieb Andrey Grodzovsky:
> 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.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzov...@amd.com>
> ---
>   drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c |  7 ++-
>   drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c | 16 +---
>   2 files changed, 7 insertions(+), 16 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..65c6ca7 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
> @@ -327,7 +327,6 @@ 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;
> @@ -345,7 +344,7 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
>sizeof(Watermarks_t),
>PAGE_SIZE,
>AMDGPU_GEM_DOMAIN_VRAM,
> - ,
> + >smu_tables.entry[WMTABLE].handle,
>_addr,
>);
>
> @@ -357,14 +356,13 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
>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,
> - ,
> + >smu_tables.entry[CLOCKTABLE].handle,
>_addr,
>);
>
> @@ -380,7 +378,6 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
>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..c8b326e 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
> @@ -377,7 +377,6 @@ 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;
> @@ -403,7 +402,7 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>sizeof(PPTable_t),
>PAGE_SIZE,
>AMDGPU_GEM_DOMAIN_VRAM,
> - ,
> + >smu_tables.entry[PPTABLE].handle,
>_addr,
>);
>
> @@ -415,14 +414,13 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>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,
> - ,
> + >smu_tables.entry[WMTABLE].handle,
>_addr,
>);
>
> @@ -434,14 +432,13 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>priv->smu_tables.entry[WMTABLE].table_id

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

2018-03-14 Thread Christian König


Am 14.03.2018 um 19:07 schrieb Andrey Grodzovsky:

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.

Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c |  7 ++-
  drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c | 16 +---
  2 files changed, 7 insertions(+), 16 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..65c6ca7 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/rv_smumgr.c
@@ -327,7 +327,6 @@ 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;
@@ -345,7 +344,7 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)
sizeof(Watermarks_t),
PAGE_SIZE,
AMDGPU_GEM_DOMAIN_VRAM,
-   ,
+   >smu_tables.entry[WMTABLE].handle,
_addr,
);
  
@@ -357,14 +356,13 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)

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,
-   ,
+   >smu_tables.entry[CLOCKTABLE].handle,
_addr,
);
  
@@ -380,7 +378,6 @@ static int rv_smu_init(struct pp_hwmgr *hwmgr)

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..c8b326e 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c
@@ -377,7 +377,6 @@ 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;
@@ -403,7 +402,7 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
sizeof(PPTable_t),
PAGE_SIZE,
AMDGPU_GEM_DOMAIN_VRAM,
-   ,
+   >smu_tables.entry[PPTABLE].handle,
_addr,
);
  
@@ -415,14 +414,13 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)

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,
-   ,
+   >smu_tables.entry[WMTABLE].handle,
_addr,
);
  
@@ -434,14 +432,13 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)

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,
-   ,
+   >smu_tables.entry[AVFSTABLE].handle,
_addr,
);
  
@@ -453,7 +450,6 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)

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;
-

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

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

2 matches

Site Navigation

Mail list logo

Footer information