[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

[PATCH v3] 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, require the offset to be aligned to the access size so the write
can never straddle a page boundary, 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>
---
v3:
 - require dst_offset to be aligned to reloc_size so the write can never
   straddle a page boundary inside the kmap window (addresses Sashiko's
   alignment finding).

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  | 23 ++++++++++++++++++++++-
 drivers/gpu/drm/qxl/qxl_object.c |  2 +-
 drivers/gpu/drm/qxl/qxl_object.h |  2 +-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026..9a031bb 100755
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -73,7 +73,7 @@ int qxl_map_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv
 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 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 	void *fb_cmd;
 	int i, ret;
 	int unwritten;
+	u32 reloc_size;
 
 	switch (cmd->type) {
 	case QXL_CMD_DRAW:
@@ -231,6 +232,26 @@ 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.  The
+		 * offset must also be aligned to the access size so the write
+		 * cannot straddle a page boundary inside the window.
+		 */
+		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
+			     sizeof(uint64_t) : sizeof(uint32_t);
+		if (!IS_ALIGNED(reloc_info[i].dst_offset, reloc_size) ||
+		    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,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 313f6c3..0df4814 100755
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -204,7 +204,7 @@ int qxl_bo_pin_and_vmap(struct qxl_bo *bo, struct iosys_map *map)
 }
 
 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;
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 875f632..a4aed9d 100755
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -63,7 +63,7 @@ int qxl_bo_pin_and_vmap(struct qxl_bo *bo, struct iosys_map *map);
 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 v3] drm/qxl: validate relocation destination offsets

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] drm/qxl: double PAGE_SIZE scaling in qxl_bo_kmap_atomic_page() fallback
- [High] drm/qxl: natural alignment check in qxl_process_single_command() breaks UAPI
- [High] drm/qxl: integer wrap and missing command bounds check for dst_handle == 0
--

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

drm/qxl: validate relocation destination offsets

This commit widens relocation destination offset types and validates
the destination offset and alignment against the destination buffer
object size in qxl_process_single_command().

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026ceff9c..9a031bb3f9b0c 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c

[ ... ]

> @@ -231,6 +232,26 @@ 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.  The
> +		 * offset must also be aligned to the access size so the write
> +		 * cannot straddle a page boundary inside the window.
> +		 */
> +		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
> +			     sizeof(uint64_t) : sizeof(uint32_t);
> +		if (!IS_ALIGNED(reloc_info[i].dst_offset, reloc_size) ||
> +		    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;
> +		}

[Severity: High]
Won't requiring dst_offset to be aligned to reloc_size (8 or 4 bytes) in
qxl_process_single_command() reject valid QXL draw commands from userspace
with -EINVAL?

In drivers/gpu/drm/qxl/qxl_dev.h, the QXL protocol structures are defined
under #pragma pack(push, 1):

drivers/gpu/drm/qxl/qxl_dev.h:
    struct qxl_drawable {
            union qxl_release_info release_info;
            uint32_t surface_id;
            uint8_t effect;
            uint8_t type;
            uint8_t self_bitmap;
            struct qxl_rect self_bitmap_area;
            struct qxl_rect bbox;
            struct qxl_clip clip;
            uint32_t mm_time;
            int32_t surfaces_dest[3];
            struct qxl_rect surfaces_rects[3];
            union {
                    struct qxl_fill fill;
                    struct qxl_opaque opaque;
                    struct qxl_copy copy;
                    ...

Because of the three uint8_t fields (effect, type, self_bitmap), subsequent
relocated fields inside struct qxl_drawable sit at unaligned byte offsets,
such as clip.data (offset 51), surfaces_dest[0..2] (offsets 63, 67, 71), and
union u fields like u.copy.src_bitmap (offset 123).

Similarly, when relocating into another buffer object (reloc.dst_handle != 0),
struct qxl_data_chunk places prev_chunk and next_chunk (8-byte QXLPHYSICAL
fields) at offsets 4 and 12:

drivers/gpu/drm/qxl/qxl_dev.h:
    struct qxl_data_chunk {
            uint32_t data_size;
            QXLPHYSICAL prev_chunk;
            QXLPHYSICAL next_chunk;
            uint8_t data[];
    };

and struct qxl_image places u.surface_image.surface_id at offset 18.

To prevent a relocation write from straddling a page boundary inside the
single page mapped by qxl_bo_kmap_atomic_page(), should this check that the
offset within the page does not cross PAGE_SIZE (for example,
offset_in_page(reloc_info[i].dst_offset) > PAGE_SIZE - reloc_size) rather
than requiring natural alignment via IS_ALIGNED()?

[Severity: High]
Also in qxl_process_single_command(), when reloc.dst_handle == 0, can
reloc.dst_offset + release->release_offset wrap around or write outside the
command's slot in cmd_bo?

In qxl_alloc_release_reserved(), cmd_bo is a PAGE_SIZE buffer object shared
across multiple 256-byte release slots at release->release_offset, with
union qxl_release_info stored at the start of each slot:

drivers/gpu/drm/qxl/qxl_release.c:qxl_alloc_release_reserved() {
    ...
    bo = qxl_bo_ref(qdev->current_release_bo[cur_idx]);

    (*release)->release_bo = bo;
    (*release)->release_offset = qdev->current_release_bo_offset[cur_idx] * release_size_per_bo[cur_idx];
    qdev->current_release_bo_offset[cur_idx]++;
    ...
}

When release->release_offset > 0, adding a large user-supplied
reloc.dst_offset can wrap around in uint64_t to a small offset before the
bounds check. Furthermore, checking only against cmd_bo->tbo.base.size
(PAGE_SIZE) instead of the command's range
[sizeof(union qxl_release_info), sizeof(union qxl_release_info) + cmd->command_size)
allows a relocation to overwrite the union qxl_release_info header or
adjacent release slots sharing cmd_bo.

>  
>  		/* 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,
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
> index 313f6c30cac85..0df48141c2db7 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -204,7 +204,7 @@ int qxl_bo_pin_and_vmap(struct qxl_bo *bo, struct iosys_map *map)
>  }
>  
>  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;

[Severity: Critical]
When dst_bo is backed by TTM_PL_SYSTEM (such as a multi-page dumb BO created
via DRM_IOCTL_MODE_CREATE_DUMB with QXL_GEM_DOMAIN_CPU), can a relocation
with dst_offset >= PAGE_SIZE still write out of bounds in the fallback path
of qxl_bo_kmap_atomic_page()?

In apply_reloc() and apply_surf_reloc(), the caller passes a byte offset
masked with PAGE_MASK and does not check the return value for NULL:

drivers/gpu/drm/qxl/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)) = qxl_bo_physical_address(qdev,
                                                                                  info->src_bo,
                                                                                  info->src_offset);
    ...
}

In qxl_bo_kmap_atomic_page(), the io_mapping_map_atomic_wc() path treats
page_offset as a byte offset, but the fallback path multiplies page_offset by
PAGE_SIZE a second time:

drivers/gpu/drm/qxl/qxl_object.c:qxl_bo_kmap_atomic_page() {
    ...
    offset = bo->tbo.resource->start << PAGE_SHIFT;
    return io_mapping_map_atomic_wc(map, offset + page_offset);
fallback:
    if (bo->kptr) {
            rptr = bo->kptr + (page_offset * PAGE_SIZE);
            return rptr;
    }

    ret = qxl_bo_vmap_locked(bo, &bo_map);
    if (ret)
            return NULL;
    rptr = bo_map.vaddr; /* TODO: Use mapping abstraction properly */

    rptr += page_offset * PAGE_SIZE;
    return rptr;
}

If dst_offset >= PAGE_SIZE (while still within dst_bo->tbo.base.size, passing
the new bounds check), doesn't page_offset * PAGE_SIZE advance rptr far past
the end of the buffer object (or return NULL if qxl_bo_vmap_locked() fails)?

[ ... ]

> -- 

—
🤖 Generated by Sashiko — Linux Kernel Patch Review
https://sashiko.dev
—

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179240154188.24976.18308245375219473922@gmail.com?part=1