Linux virtualization list
 help / color / mirror / Atom feed
From: Zhou Qiuyue <n0ps1edzz@gmail.com>
To: Dave Airlie <airlied@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
Cc: dri-devel@lists.freedesktop.org,
	spice-devel@lists.freedesktop.org,
	virtualization@lists.linux.dev
Subject: [PATCH v2] drm/qxl: validate relocation destination offsets
Date: Sat, 10 Oct 2026 20:01:23 -0700 (PDT)	[thread overview]
Message-ID: <6acafc03.6b76d992.279a8.b343@mx.google.com> (raw)

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);


             reply	other threads:[~2026-10-11  3:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-11  3:01 Zhou Qiuyue [this message]
2026-10-11  3:17 ` [PATCH v2] drm/qxl: validate relocation destination offsets sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6acafc03.6b76d992.279a8.b343@mx.google.com \
    --to=n0ps1edzz@gmail.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kraxel@redhat.com \
    --cc=spice-devel@lists.freedesktop.org \
    --cc=virtualization@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox