[PATCH] drm/qxl: validate dst_offset in apply_reloc against BO size

[PATCH] drm/qxl: validate dst_offset in apply_reloc against BO size

[Berkant Koc]

The QXL DRM relocation handlers apply_reloc() and apply_surf_reloc() in
drivers/gpu/drm/qxl/qxl_ioctl.c perform user-controlled writes at
reloc_page + (info->dst_offset & ~PAGE_MASK), where dst_offset comes
verbatim from the drm_qxl_reloc ioctl argument. An aspirational comment
above apply_reloc explicitly states "dst must be validated, i.e. whole
bo on vram/surfacesram", but the validation has never been implemented
since the driver was merged in v3.10 (2013).

qxl_process_single_command() copies the user drm_qxl_reloc array into
qxl_reloc_info, narrowing dst_offset from __u64 to uint32_t. This still
gives the caller (any DRM_AUTH client) the full 32-bit / 4 GiB range and
no bounds check against dst_bo->tbo.base.size happens before the writes.

The kmap target is io_mapping_map_atomic_wc() over the QXL device's full
VRAM aperture (vram_mapping). The BUG_ON inside io-mapping.h only fires
when the offset exceeds the entire aperture, not the specific BO, so
writes within the aperture but outside the resolved BO succeed and land
on whatever data occupies those pages -- including other guest processes'
QXL BOs, queued QXL release-command BOs, and ring buffers that QEMU
consumes.

The kernel-side primitive is a guest-local OOB write (CWE-787) against
guest VRAM, not a host escape. Corrupting QXL command structures that
QEMU/SPICE then parses moves the attack surface to QEMU's QXL parser
(separate codebase, historically vulnerable), so this is a viable
priming primitive for guest-to-host chains whose terminal step lives in
userspace QEMU. The kernel bug stands as a defense-in-depth concern
independent of any QEMU chain.

Public discussion of this issue is at:
https://lore.kernel.org/virtualization/36acd33982bfdce04090e17294596ff8@berkoc.com/T/#u

Add an explicit bound check in the per-reloc fill loop in
qxl_process_single_command() so that both apply_reloc (8-byte write) and
apply_surf_reloc (4-byte write) refuse out-of-BO offsets and the
arithmetic overflow case (dst_offset close to U32_MAX) with -EINVAL.

Signed-off-by: Berkant Koc <me@berkoc.com>
---
 drivers/gpu/drm/qxl/qxl_ioctl.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026..c65d468 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -231,6 +231,26 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
 		}
 
+		/*
+		 * dst must be validated, i.e. whole bo on vram/surfacesram.
+		 * Refuse offsets that would write past the end of the bo, or
+		 * where dst_offset + write_size would overflow uint32_t.
+		 */
+		if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
+		    (reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t) ||
+		     reloc_info[i].dst_offset + sizeof(uint64_t) >
+		     reloc_info[i].dst_bo->tbo.base.size)) {
+			ret = -EINVAL;
+			goto out_free_bos;
+		}
+		if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
+		    (reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t) ||
+		     reloc_info[i].dst_offset + sizeof(uint32_t) >
+		     reloc_info[i].dst_bo->tbo.base.size)) {
+			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,
-- 
2.43.0