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

Zhu, Rex Rex.Zhu at amd.com
Thu Mar 15 09:36:57 UTC 2018


Apply this patch with Christian's suggestions.


Thanks.


Best Regards

Rex


________________________________
From: Christian König <ckoenig.leichtzumerken at gmail.com>
Sent: Thursday, March 15, 2018 2:21 AM
To: Grodzovsky, Andrey; amd-gfx at 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.grodzovsky at 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,
> -                     &handle,
> +                     &priv->smu_tables.entry[WMTABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>
> @@ -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,
> -                     &handle,
> +                     &priv->smu_tables.entry[CLOCKTABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>
> @@ -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,
> -                     &handle,
> +                     &priv->smu_tables.entry[PPTABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>
> @@ -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,
> -                     &handle,
> +                     &priv->smu_tables.entry[WMTABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>
> @@ -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,
> -                     &handle,
> +                     &priv->smu_tables.entry[AVFSTABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>
> @@ -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;
> -     priv->smu_tables.entry[AVFSTABLE].handle = handle;
>
>        tools_size = 0x19000;
>        if (tools_size) {
> @@ -461,7 +457,7 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>                                tools_size,
>                                PAGE_SIZE,
>                                AMDGPU_GEM_DOMAIN_VRAM,
> -                             &handle,
> +                             &priv->smu_tables.entry[TOOLSTABLE].handle,
>                                &mc_addr,
>                                &kaddr);

I think it would be better to specify mc_addr and kaddr directly as well.

Christian.

>                if (ret)
> @@ -471,7 +467,6 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>                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,7 +474,7 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>                        sizeof(AvfsFuseOverride_t),
>                        PAGE_SIZE,
>                        AMDGPU_GEM_DOMAIN_VRAM,
> -                     &handle,
> +                     &priv->smu_tables.entry[AVFSFUSETABLE].handle,
>                        &mc_addr,
>                        &kaddr);
>        if (ret)
> @@ -490,7 +485,6 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>        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;
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180315/8e2b21e4/attachment-0001.html>


More information about the amd-gfx mailing list