[PATCH] accel/amdxdna: Validate sync BO range before flushing

[PATCH] accel/amdxdna: Validate sync BO range before flushing

[Shuvam Pandey]

amdxdna_drm_sync_bo_ioctl() takes offset and size from userspace and
uses them to build the address and length passed to
drm_clflush_virt_range() for BOs that have a kernel mapping. The values
are not checked against the BO size, so an out-of-range request can pass
an address outside the BO mapping to cache maintenance.

Reject ranges outside the GEM object. Treat zero-length ranges as a no-op
before cache maintenance, because drm_clflush_virt_range() still flushes
end - 1 on x86. Leave the existing SYNC_DIRECT_FROM_DEVICE debug-buffer
sync behavior unchanged.

Fixes: d76856beb4a4 ("accel/amdxdna: Refactor GEM BO handling and add helper APIs for address retrieval")
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 6e367ddb9e1b..c56c8fd86276 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1040,20 +1040,33 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
 	}
 	abo = to_xdna_obj(gobj);
 
+	if (args->offset > gobj->size ||
+	    args->size > gobj->size - args->offset) {
+		ret = -EINVAL;
+		goto put_obj;
+	}
+
 	ret = amdxdna_gem_pin(abo);
 	if (ret) {
 		XDNA_ERR(xdna, "Pin BO %d failed, ret %d", args->handle, ret);
 		goto put_obj;
 	}
 
-	if (is_import_bo(abo))
-		drm_clflush_sg(abo->base.sgt);
-	else if (amdxdna_gem_vmap(abo))
-		drm_clflush_virt_range(amdxdna_gem_vmap(abo) + args->offset, args->size);
-	else if (abo->base.pages)
-		drm_clflush_pages(abo->base.pages, gobj->size >> PAGE_SHIFT);
-	else
-		drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
+	if (args->size) {
+		if (is_import_bo(abo)) {
+			drm_clflush_sg(abo->base.sgt);
+		} else {
+			void *kva = amdxdna_gem_vmap(abo);
+
+			if (kva)
+				drm_clflush_virt_range(kva + args->offset,
+						       args->size);
+			else if (abo->base.pages)
+				drm_clflush_pages(abo->base.pages, gobj->size >> PAGE_SHIFT);
+			else
+				drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
+		}
+	}
 
 	amdxdna_gem_unpin(abo);
 

Re: [PATCH] accel/amdxdna: Validate sync BO range before flushing

[Lizhi Hou]

Hi Shuvam,


Thanks for providing the patch. This is going to be fixed by

https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/dbc8fd7a03cbc0704e8e558a448015f620547a02


Lizhi

On 6/13/26 14:37, Shuvam Pandey wrote:
> amdxdna_drm_sync_bo_ioctl() takes offset and size from userspace and
> uses them to build the address and length passed to
> drm_clflush_virt_range() for BOs that have a kernel mapping. The values
> are not checked against the BO size, so an out-of-range request can pass
> an address outside the BO mapping to cache maintenance.
>
> Reject ranges outside the GEM object. Treat zero-length ranges as a no-op
> before cache maintenance, because drm_clflush_virt_range() still flushes
> end - 1 on x86. Leave the existing SYNC_DIRECT_FROM_DEVICE debug-buffer
> sync behavior unchanged.
>
> Fixes: d76856beb4a4 ("accel/amdxdna: Refactor GEM BO handling and add helper APIs for address retrieval")
> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 6e367ddb9e1b..c56c8fd86276 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1040,20 +1040,33 @@ int amdxdna_drm_sync_bo_ioctl(struct drm_device *dev,
>   	}
>   	abo = to_xdna_obj(gobj);
>   
> +	if (args->offset > gobj->size ||
> +	    args->size > gobj->size - args->offset) {
> +		ret = -EINVAL;
> +		goto put_obj;
> +	}
> +
>   	ret = amdxdna_gem_pin(abo);
>   	if (ret) {
>   		XDNA_ERR(xdna, "Pin BO %d failed, ret %d", args->handle, ret);
>   		goto put_obj;
>   	}
>   
> -	if (is_import_bo(abo))
> -		drm_clflush_sg(abo->base.sgt);
> -	else if (amdxdna_gem_vmap(abo))
> -		drm_clflush_virt_range(amdxdna_gem_vmap(abo) + args->offset, args->size);
> -	else if (abo->base.pages)
> -		drm_clflush_pages(abo->base.pages, gobj->size >> PAGE_SHIFT);
> -	else
> -		drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
> +	if (args->size) {
> +		if (is_import_bo(abo)) {
> +			drm_clflush_sg(abo->base.sgt);
> +		} else {
> +			void *kva = amdxdna_gem_vmap(abo);
> +
> +			if (kva)
> +				drm_clflush_virt_range(kva + args->offset,
> +						       args->size);
> +			else if (abo->base.pages)
> +				drm_clflush_pages(abo->base.pages, gobj->size >> PAGE_SHIFT);
> +			else
> +				drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
> +		}
> +	}
>   
>   	amdxdna_gem_unpin(abo);
>