[PATCH v2] drm/qxl: validate relocation destination offsets

[PATCH v2] drm/qxl: validate relocation destination offsets

[Zhou Qiuyue]

The QXL execbuffer ioctl lets userspace request relocations into a
destination buffer object at an arbitrary offset.  The offset is never
validated against the size of that object, and the relocation is applied
with a plain pointer write into the object's io-mapping window:

  apply_reloc():
      reloc_page = qxl_bo_kmap_atomic_page(qdev, dst_bo, dst_offset & PAGE_MASK);
      *(uint64_t *)(reloc_page + (dst_offset & ~PAGE_MASK)) = ...;

  apply_surf_reloc():
      reloc_page = qxl_bo_kmap_atomic_page(qdev, dst_bo, dst_offset & PAGE_MASK);
      *(uint32_t *)(reloc_page + (dst_offset & ~PAGE_MASK)) = id;

so a user supplied dst_offset turns into an out-of-bounds kernel write of
8 (or 4) bytes.  As the offset is additionally truncated into a 32bit
member and handed on as "int page_offset" to qxl_bo_kmap_atomic_page(),
the write can even be placed before the object (sign extension).

Validate the offset plus the access size against the destination object
size, and make the offset/parameter types wide enough that the check is
not defeated by truncation or sign extension.

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Cc: stable@vger.kernel.org
Signed-off-by: Zhou Qiuyue <n0ps1edzz@gmail.com>
---
v2: use a real-name Signed-off-by (noted by Willy Tarreau).
    The reproducer and full report were sent to the maintainers on 2026-10-06;
    the reproducer can be shared on request.
 drivers/gpu/drm/qxl/qxl_ioctl.c  | 20 +++++++++++++++++++-
 drivers/gpu/drm/qxl/qxl_object.c |  2 +-
 drivers/gpu/drm/qxl/qxl_object.h |  2 +-
 3 files changed, 21 insertions(+), 3 deletions(-)

--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -73,7 +73,7 @@
 struct qxl_reloc_info {
 	int type;
 	struct qxl_bo *dst_bo;
-	uint32_t dst_offset;
+	uint64_t dst_offset;
 	struct qxl_bo *src_bo;
 	int src_offset;
 };
@@ -149,6 +149,7 @@
 	void *fb_cmd;
 	int i, ret;
 	int unwritten;
+	u32 reloc_size;
 
 	switch (cmd->type) {
 	case QXL_CMD_DRAW:
@@ -231,6 +232,23 @@
 			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
 		}
 
+		/*
+		 * The relocation is applied with a plain write into the memory
+		 * window of the destination object, so the user supplied offset
+		 * must be validated against the object size.  Otherwise userspace
+		 * can write anywhere in the mapping window set up by
+		 * qxl_bo_kmap_atomic_page(), i.e. outside of the object.
+		 */
+		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
+			     sizeof(uint64_t) : sizeof(uint32_t);
+		if (reloc_info[i].dst_offset > reloc_info[i].dst_bo->tbo.base.size ||
+		    reloc_size > reloc_info[i].dst_bo->tbo.base.size -
+				 reloc_info[i].dst_offset) {
+			DRM_DEBUG("reloc dst offset out of bounds\n");
+			ret = -EINVAL;
+			goto out_free_bos;
+		}
+
 		/* reserve and validate the reloc dst bo */
 		if (reloc.reloc_type == QXL_RELOC_TYPE_BO || reloc.src_handle) {
 			ret = qxlhw_handle_to_bo(file_priv, reloc.src_handle, release,
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -204,7 +204,7 @@
 }
 
 void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
-			      struct qxl_bo *bo, int page_offset)
+			      struct qxl_bo *bo, unsigned long page_offset)
 {
 	unsigned long offset;
 	void *rptr;
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -63,7 +63,7 @@
 int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map *map);
 int qxl_bo_vunmap_and_unpin(struct qxl_bo *bo);
 void qxl_bo_vunmap_locked(struct qxl_bo *bo);
-void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset);
+void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, unsigned long page_offset);
 void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map);
 extern struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo);
 extern void qxl_bo_unref(struct qxl_bo **bo);

Re: [PATCH v2] drm/qxl: validate relocation destination offsets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] qxl: unaligned dst_offset causes page-boundary cross and alignment faults
--

commit 214f69f63b016ba44fb4af3f2bea1273b9aa972b
Author: Zhou Qiuyue <n0ps1edzz@gmail.com>

drm/qxl: validate relocation destination offsets

This patch validates the user-supplied relocation offset against the
destination object's size to prevent out-of-bounds kernel writes. It also
widens data types to prevent truncation and sign-extension vulnerabilities.

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026ceff9c..ae32295fd5e84 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
[ ... ]
> @@ -231,6 +232,23 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>  			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
>  		}
>  
> +		/*
> +		 * The relocation is applied with a plain write into the memory
> +		 * window of the destination object, so the user supplied offset
> +		 * must be validated against the object size.  Otherwise userspace
> +		 * can write anywhere in the mapping window set up by
> +		 * qxl_bo_kmap_atomic_page(), i.e. outside of the object.
> +		 */
> +		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
> +			     sizeof(uint64_t) : sizeof(uint32_t);
> +		if (reloc_info[i].dst_offset > reloc_info[i].dst_bo->tbo.base.size ||
> +		    reloc_size > reloc_info[i].dst_bo->tbo.base.size -
> +				 reloc_info[i].dst_offset) {

[Severity: High]
Does this bounds check ensure that dst_offset is properly aligned to
reloc_size?

If userspace passes an unaligned dst_offset, could the pointer write in
apply_reloc() cross a page boundary?

In qxl_ioctl.c:apply_reloc():
    reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo,
                                         info->dst_offset & PAGE_MASK);
    *(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = ...;

If (dst_offset & ~PAGE_MASK) > PAGE_SIZE - 8, the 8-byte write could cross
into an unmapped or adjacent page, potentially causing a fault when using
CONFIG_HAVE_ATOMIC_IOMAP.

Additionally, could the unaligned cast to (uint64_t *) trigger unaligned access
exceptions on architectures with strict alignment requirements?

> +			DRM_DEBUG("reloc dst offset out of bounds\n");
> +			ret = -EINVAL;
> +			goto out_free_bos;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6acafc03.6b76d992.279a8.b343@mx.google.com?part=1