[PATCH V2 1/2] drm/amdgpu/userq: only accept doorbell BOs as queue doorbell

[PATCH V2 1/2] drm/amdgpu/userq: only accept doorbell BOs as queue doorbell

[Jesse Zhang]

amdgpu_userq_get_doorbell_index() pins any BO passed as doorbell_handle
into the doorbell domain. For a regular BO, TTM moves it with
ttm_bo_move_null() and its contents are lost. Since an importer on the
same device gets the exporter's GEM object, a client can do this to a
buffer another client shared with it.

Reject BOs that were not created in the doorbell domain.

v2: drop the error message, use a local abo variable (Christian)

Fixes: f09c1e6077ab ("drm/amdgpu: generate doorbell index for userqueue")
Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 89e03d3a930c..5a922f283743 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -469,6 +469,7 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
 	u64 doorbell_index;
 	struct drm_gem_object *gobj;
 	struct amdgpu_userq_obj *db_obj = db_info->db_obj;
+	struct amdgpu_bo *abo;
 	int r, db_size;
 
 	gobj = drm_gem_object_lookup(filp, db_info->doorbell_handle);
@@ -477,7 +478,17 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
 		return -EINVAL;
 	}
 
-	db_obj->obj = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj));
+	/*
+	 * Pinning a regular BO into the doorbell domain would discard its
+	 * contents, possibly those of a buffer shared by another client.
+	 */
+	abo = gem_to_amdgpu_bo(gobj);
+	if (!(abo->preferred_domains & AMDGPU_GEM_DOMAIN_DOORBELL)) {
+		drm_gem_object_put(gobj);
+		return -EINVAL;
+	}
+
+	db_obj->obj = amdgpu_bo_ref(abo);
 	drm_gem_object_put(gobj);
 
 	r = amdgpu_bo_reserve(db_obj->obj, true);
-- 
2.50.1

[PATCH V2 2/2] drm/amdgpu/userq: return the memdup_user() error for the user MQD

[Jesse Zhang]

mes_userq_mqd_create() returns -ENOMEM when copying the user MQD fails,
even though memdup_user() returns -EFAULT for a bad pointer or -EINTR
when interrupted by a signal. Return PTR_ERR() instead.

While at it drop the error messages. Neither case is a driver error and
userspace shouldn't be able to spam the log with invalid IOCTL
parameters.

v2: drop the error messages (Christian)

Fixes: 2c695d7c0720 ("drm/amdgpu: enable compute/gfx usermode queue")
Fixes: 543b61453774 ("drm/amdgpu: enable SDMA usermode queues")
Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
index e6c167a39d44..1b76c10d1ac2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
@@ -407,8 +407,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 
 		compute_mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
 		if (IS_ERR(compute_mqd)) {
-			DRM_ERROR("Failed to read user MQD\n");
-			r = -ENOMEM;
+			r = PTR_ERR(compute_mqd);
 			goto free_mqd;
 		}
 
@@ -452,8 +451,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 
 		mqd_gfx_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
 		if (IS_ERR(mqd_gfx_v11)) {
-			DRM_ERROR("Failed to read user MQD\n");
-			r = -ENOMEM;
+			r = PTR_ERR(mqd_gfx_v11);
 			goto free_mqd;
 		}
 
@@ -497,8 +495,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 
 		mqd_sdma_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
 		if (IS_ERR(mqd_sdma_v11)) {
-			DRM_ERROR("Failed to read sdma user MQD\n");
-			r = -ENOMEM;
+			r = PTR_ERR(mqd_sdma_v11);
 			goto free_mqd;
 		}
 
-- 
2.50.1

Re: [PATCH V2 1/2] drm/amdgpu/userq: only accept doorbell BOs as queue doorbell

[Christian König]

On 9/30/26 11:38, Jesse Zhang wrote:
> amdgpu_userq_get_doorbell_index() pins any BO passed as doorbell_handle
> into the doorbell domain. For a regular BO, TTM moves it with
> ttm_bo_move_null() and its contents are lost. Since an importer on the
> same device gets the exporter's GEM object, a client can do this to a
> buffer another client shared with it.
> 
> Reject BOs that were not created in the doorbell domain.
> 
> v2: drop the error message, use a local abo variable (Christian)
> 
> Fixes: f09c1e6077ab ("drm/amdgpu: generate doorbell index for userqueue")
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 89e03d3a930c..5a922f283743 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -469,6 +469,7 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>  	u64 doorbell_index;
>  	struct drm_gem_object *gobj;
>  	struct amdgpu_userq_obj *db_obj = db_info->db_obj;
> +	struct amdgpu_bo *abo;
>  	int r, db_size;
>  
>  	gobj = drm_gem_object_lookup(filp, db_info->doorbell_handle);
> @@ -477,7 +478,17 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>  		return -EINVAL;
>  	}
>  
> -	db_obj->obj = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj));
> +	/*
> +	 * Pinning a regular BO into the doorbell domain would discard its
> +	 * contents, possibly those of a buffer shared by another client.
> +	 */
> +	abo = gem_to_amdgpu_bo(gobj);
> +	if (!(abo->preferred_domains & AMDGPU_GEM_DOMAIN_DOORBELL)) {
> +		drm_gem_object_put(gobj);
> +		return -EINVAL;
> +	}
> +
> +	db_obj->obj = amdgpu_bo_ref(abo);
>  	drm_gem_object_put(gobj);
>  
>  	r = amdgpu_bo_reserve(db_obj->obj, true);

Re: [PATCH V2 2/2] drm/amdgpu/userq: return the memdup_user() error for the user MQD

[Christian König]

On 9/30/26 11:38, Jesse Zhang wrote:
> mes_userq_mqd_create() returns -ENOMEM when copying the user MQD fails,
> even though memdup_user() returns -EFAULT for a bad pointer or -EINTR
> when interrupted by a signal. Return PTR_ERR() instead.
> 
> While at it drop the error messages. Neither case is a driver error and
> userspace shouldn't be able to spam the log with invalid IOCTL
> parameters.
> 
> v2: drop the error messages (Christian)
> 
> Fixes: 2c695d7c0720 ("drm/amdgpu: enable compute/gfx usermode queue")
> Fixes: 543b61453774 ("drm/amdgpu: enable SDMA usermode queues")
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> index e6c167a39d44..1b76c10d1ac2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> @@ -407,8 +407,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
>  
>  		compute_mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
>  		if (IS_ERR(compute_mqd)) {
> -			DRM_ERROR("Failed to read user MQD\n");
> -			r = -ENOMEM;
> +			r = PTR_ERR(compute_mqd);
>  			goto free_mqd;
>  		}
>  
> @@ -452,8 +451,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
>  
>  		mqd_gfx_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
>  		if (IS_ERR(mqd_gfx_v11)) {
> -			DRM_ERROR("Failed to read user MQD\n");
> -			r = -ENOMEM;
> +			r = PTR_ERR(mqd_gfx_v11);
>  			goto free_mqd;
>  		}
>  
> @@ -497,8 +495,7 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
>  
>  		mqd_sdma_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size);
>  		if (IS_ERR(mqd_sdma_v11)) {
> -			DRM_ERROR("Failed to read sdma user MQD\n");
> -			r = -ENOMEM;
> +			r = PTR_ERR(mqd_sdma_v11);
>  			goto free_mqd;
>  		}
>