Re: [PATCH] drm/amdgpu: fix amdgpu_cs_p1_user_fence

[Alex Deucher]

On Tue, Aug 29, 2023 at 8:00 AM Christian König
 wrote:
>
> The offset is just 32bits here so this can potentially overflow if
> somebody specifies a large value. Instead reduce the size to calculate
> the last possible offset.
>
> The error handling path incorrectly drops the reference to the user
> fence BO resulting in potential reference count underflow.
>
> Signed-off-by: Christian König 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 17 -
>  1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index f4b5572c54f2..5c8729491105 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -139,23 +139,14 @@ static int amdgpu_cs_p1_user_fence(struct 
> amdgpu_cs_parser *p,
> drm_gem_object_put(gobj);
>
> size = amdgpu_bo_size(bo);
> -   if (size != PAGE_SIZE || (data->offset + 8) > size) {
> -   r = -EINVAL;
> -   goto error_unref;
> -   }
> +   if (size != PAGE_SIZE || data->offset > (size - 8))
> +   return -EINVAL;
>
> -   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm)) {
> -   r = -EINVAL;
> -   goto error_unref;
> -   }
> +   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm))
> +   return -EINVAL;
>
> *offset = data->offset;
> -
> return 0;
> -
> -error_unref:
> -   amdgpu_bo_unref();
> -   return r;
>  }
>
>  static int amdgpu_cs_p1_bo_handles(struct amdgpu_cs_parser *p,
> --
> 2.34.1
>

[PATCH] drm/amdgpu: fix amdgpu_cs_p1_user_fence

[Christian König]

The offset is just 32bits here so this can potentially overflow if
somebody specifies a large value. Instead reduce the size to calculate
the last possible offset.

The error handling path incorrectly drops the reference to the user
fence BO resulting in potential reference count underflow.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index f4b5572c54f2..5c8729491105 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -139,23 +139,14 @@ static int amdgpu_cs_p1_user_fence(struct 
amdgpu_cs_parser *p,
drm_gem_object_put(gobj);
 
size = amdgpu_bo_size(bo);
-   if (size != PAGE_SIZE || (data->offset + 8) > size) {
-   r = -EINVAL;
-   goto error_unref;
-   }
+   if (size != PAGE_SIZE || data->offset > (size - 8))
+   return -EINVAL;
 
-   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm)) {
-   r = -EINVAL;
-   goto error_unref;
-   }
+   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm))
+   return -EINVAL;
 
*offset = data->offset;
-
return 0;
-
-error_unref:
-   amdgpu_bo_unref();
-   return r;
 }
 
 static int amdgpu_cs_p1_bo_handles(struct amdgpu_cs_parser *p,
-- 
2.34.1