[PATCH v2] drm/amdgpu: validate GEM_CREATE domain combinations

[PATCH v2] drm/amdgpu: validate GEM_CREATE domain combinations

[Candice Li]

AMDGPU_GEM_CREATE checked domain bits against AMDGPU_GEM_DOMAIN_MASK,
but did not validate domain combinations. Userspace could combine
CPU|GTT|VRAM with DOORBELL, GDS, GWS, or OA, making
amdgpu_bo_placement_from_domain() exceed AMDGPU_BO_MAX_PLACEMENTS and
hit BUG_ON().

Allow combinations only within CPU/GTT/VRAM, and require non-CPU/GTT/
VRAM domains to be specified one at a time. Return -EINVAL for invalid
combinations in amdgpu_gem_create_ioctl().

v2: Rename helper from amdgpu_gem_domain_valid() to
    amdgpu_gem_are_domains_valid() (Christian)

Signed-off-by: Candice Li <candice.li@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 6a0699746fbcd6..f754a4a3a1c22d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -397,6 +397,25 @@ const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
 	.vm_ops = &amdgpu_gem_vm_ops,
 };
 
+static bool amdgpu_gem_are_domains_valid(u32 domains)
+{
+	u32 normal = AMDGPU_GEM_DOMAIN_CPU |
+		     AMDGPU_GEM_DOMAIN_GTT |
+		     AMDGPU_GEM_DOMAIN_VRAM;
+	/* Treat all non CPU/GTT/VRAM domains as special domains. */
+	u32 special = AMDGPU_GEM_DOMAIN_MASK & ~normal;
+	u32 normal_mask = domains & normal;
+	u32 special_mask = domains & special;
+
+	if (!special_mask)
+		return true;
+
+	if (normal_mask)
+		return false;
+
+	return !(special_mask & (special_mask - 1));
+}
+
 /*
  * GEM ioctls.
  */
@@ -421,6 +440,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
 	/* reject invalid gem domains */
 	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
 		return -EINVAL;
+	if (!amdgpu_gem_are_domains_valid(args->in.domains))
+		return -EINVAL;
 
 	if (!amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
 		DRM_NOTE_ONCE("Cannot allocate secure buffer since TMZ is disabled\n");
-- 
2.50.1

Re: [PATCH v2] drm/amdgpu: validate GEM_CREATE domain combinations

[Christian König]

On 8/6/26 12:00, Candice Li wrote:
> AMDGPU_GEM_CREATE checked domain bits against AMDGPU_GEM_DOMAIN_MASK,
> but did not validate domain combinations. Userspace could combine
> CPU|GTT|VRAM with DOORBELL, GDS, GWS, or OA, making
> amdgpu_bo_placement_from_domain() exceed AMDGPU_BO_MAX_PLACEMENTS and
> hit BUG_ON().
> 
> Allow combinations only within CPU/GTT/VRAM, and require non-CPU/GTT/
> VRAM domains to be specified one at a time. Return -EINVAL for invalid
> combinations in amdgpu_gem_create_ioctl().
> 
> v2: Rename helper from amdgpu_gem_domain_valid() to
>     amdgpu_gem_are_domains_valid() (Christian)
> 
> Signed-off-by: Candice Li <candice.li@amd.com>

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

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6a0699746fbcd6..f754a4a3a1c22d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -397,6 +397,25 @@ const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>  	.vm_ops = &amdgpu_gem_vm_ops,
>  };
>  
> +static bool amdgpu_gem_are_domains_valid(u32 domains)
> +{
> +	u32 normal = AMDGPU_GEM_DOMAIN_CPU |
> +		     AMDGPU_GEM_DOMAIN_GTT |
> +		     AMDGPU_GEM_DOMAIN_VRAM;
> +	/* Treat all non CPU/GTT/VRAM domains as special domains. */
> +	u32 special = AMDGPU_GEM_DOMAIN_MASK & ~normal;
> +	u32 normal_mask = domains & normal;
> +	u32 special_mask = domains & special;
> +
> +	if (!special_mask)
> +		return true;
> +
> +	if (normal_mask)
> +		return false;
> +
> +	return !(special_mask & (special_mask - 1));
> +}
> +
>  /*
>   * GEM ioctls.
>   */
> @@ -421,6 +440,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>  	/* reject invalid gem domains */
>  	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>  		return -EINVAL;
> +	if (!amdgpu_gem_are_domains_valid(args->in.domains))
> +		return -EINVAL;
>  
>  	if (!amdgpu_is_tmz(adev) && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
>  		DRM_NOTE_ONCE("Cannot allocate secure buffer since TMZ is disabled\n");