[PATCH] drm/amdgpu: fixx NULL pointer deref in gmc_v9_0_get_vm_pte

[PATCH] drm/amdgpu: fixx NULL pointer deref in gmc_v9_0_get_vm_pte

[Christian König]

We not only need to make sure that we have a BO, but also that the BO
has some backing store.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 50386eb2eec8..afc0cfed5065 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1185,6 +1185,8 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
 				struct amdgpu_bo_va_mapping *mapping,
 				uint64_t *flags)
 {
+	struct amdgpu_bo *bo = mapping->bo_va->base.bo;
+
 	*flags &= ~AMDGPU_PTE_EXECUTABLE;
 	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
 
@@ -1196,7 +1198,7 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
 		*flags &= ~AMDGPU_PTE_VALID;
 	}
 
-	if (mapping->bo_va->base.bo)
+	if (bo && bo->resource)
 		gmc_v9_0_get_coherence_flags(adev, mapping->bo_va->base.bo,
 					     mapping, flags);
 }
-- 
2.34.1

Re: [PATCH] drm/amdgpu: fixx NULL pointer deref in gmc_v9_0_get_vm_pte

[Alex Deucher]

On Wed, Dec 7, 2022 at 2:49 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> We not only need to make sure that we have a BO, but also that the BO
> has some backing store.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 50386eb2eec8..afc0cfed5065 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1185,6 +1185,8 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>                                 struct amdgpu_bo_va_mapping *mapping,
>                                 uint64_t *flags)
>  {
> +       struct amdgpu_bo *bo = mapping->bo_va->base.bo;
> +
>         *flags &= ~AMDGPU_PTE_EXECUTABLE;
>         *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>
> @@ -1196,7 +1198,7 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>                 *flags &= ~AMDGPU_PTE_VALID;
>         }
>
> -       if (mapping->bo_va->base.bo)
> +       if (bo && bo->resource)
>                 gmc_v9_0_get_coherence_flags(adev, mapping->bo_va->base.bo,
>                                              mapping, flags);
>  }
> --
> 2.34.1
>

Re: [PATCH] drm/amdgpu: fixx NULL pointer deref in gmc_v9_0_get_vm_pte

[Luben Tuikov]

Right--I just double checked with the koops and it is exactly the "resource" pointer
in the tbo which is NULL.

Reviewed-by:Luben Tuikov <luben.tuikov@amd.com>

Regards,
Luben

On 2022-12-07 02:49, Christian König wrote:
> We not only need to make sure that we have a BO, but also that the BO
> has some backing store.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 50386eb2eec8..afc0cfed5065 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1185,6 +1185,8 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>  				struct amdgpu_bo_va_mapping *mapping,
>  				uint64_t *flags)
>  {
> +	struct amdgpu_bo *bo = mapping->bo_va->base.bo;
> +
>  	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>  	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>  
> @@ -1196,7 +1198,7 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>  		*flags &= ~AMDGPU_PTE_VALID;
>  	}
>  
> -	if (mapping->bo_va->base.bo)
> +	if (bo && bo->resource)
>  		gmc_v9_0_get_coherence_flags(adev, mapping->bo_va->base.bo,
>  					     mapping, flags);
>  }

Re: [PATCH] drm/amdgpu: fixx NULL pointer deref in gmc_v9_0_get_vm_pte

[Felix Kuehling]

Am 2022-12-07 um 02:49 schrieb Christian König:
> We not only need to make sure that we have a BO, but also that the BO
> has some backing store.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Mea culpa.

Fixes: d1a372af1c3d ("drm/amdgpu: Set MTYPE in PTE based on BO flags")

(using the commit hash from drm-next, not amd-staging-drm-next) Other 
than that, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 50386eb2eec8..afc0cfed5065 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1185,6 +1185,8 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>   				struct amdgpu_bo_va_mapping *mapping,
>   				uint64_t *flags)
>   {
> +	struct amdgpu_bo *bo = mapping->bo_va->base.bo;
> +
>   	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>   	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>   
> @@ -1196,7 +1198,7 @@ static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>   		*flags &= ~AMDGPU_PTE_VALID;
>   	}
>   
> -	if (mapping->bo_va->base.bo)
> +	if (bo && bo->resource)
>   		gmc_v9_0_get_coherence_flags(adev, mapping->bo_va->base.bo,
>   					     mapping, flags);
>   }