[PATCH] drm/qxl: fix vmalloc OOB write, surface size overflow, and BO reloc leaks

[Hui Peng <benquike@gmail.com>]

Fix multiple memory safety and resource management bugs in the QXL ioctl
and buffer object paths:

1. In qxl_bo_kmap_atomic_page(), page_offset is already a byte offset
   (reloc_info->dst_offset & PAGE_MASK), yet the fallback kptr and
   ttm_bo_vmap paths multiply page_offset by PAGE_SIZE a second time,
   causing a +16 MiB out-of-bounds kernel vmalloc write when applying
   relocations in qxl_process_single_command(). Add page_offset directly
   and validate reloc.dst_offset against dst_bo->tbo.base.size and
   cmd->command_size.
2. In qxl_alloc_surf_ioctl(), param->stride * param->height is computed
   using 32-bit signed arithmetic and param->stride == INT_MIN overflows
   on negation, allowing a 4 GiB surface to wrap to a 4 KiB GEM BO. Use
   check_mul_overflow() and check_add_overflow() with size_t.
3. In qxl_process_single_command(), prevent overwriting the union
   qxl_release_info header at offset 0 of cmd_bo, and reserve/unreserve
   non-command dst_bo buffers around apply_reloc()/apply_surf_reloc().
4. In qxl_bo_create(), reject size == 0 or size > ULONG_MAX - PAGE_SIZE + 1
   before roundup(), and in qxl_bo_check_id(), deallocate bo->surface_id
   if qxl_hw_surface_alloc() fails.

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index cc02b5f10ad9..4978208ce80b 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -297,7 +297,7 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev);
 /* qxl_gem.c */
 void qxl_gem_init(struct qxl_device *qdev);
 void qxl_gem_fini(struct qxl_device *qdev);
-int qxl_gem_object_create(struct qxl_device *qdev, int size,
+int qxl_gem_object_create(struct qxl_device *qdev, size_t size,
 			  int alignment, int initial_domain,
 			  bool discardable, bool kernel,
 			  struct qxl_surface *surf,
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index 4939b57a2a48..bcd4d0b6c4fc 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -43,7 +43,7 @@ void qxl_gem_object_free(struct drm_gem_object *gobj)
 	ttm_bo_fini(tbo);
 }
 
-int qxl_gem_object_create(struct qxl_device *qdev, int size,
+int qxl_gem_object_create(struct qxl_device *qdev, size_t size,
 			  int alignment, int initial_domain,
 			  bool discardable, bool kernel,
 			  struct qxl_surface *surf,
@@ -60,7 +60,7 @@ int qxl_gem_object_create(struct qxl_device *qdev, int size,
 	if (r) {
 		if (r != -ERESTARTSYS)
 			DRM_ERROR(
-			"Failed to allocate GEM object (%d, %d, %u, %d)\n",
+			"Failed to allocate GEM object (%zu, %d, %u, %d)\n",
 				  size, initial_domain, alignment, r);
 		return r;
 	}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026ceff9..6bb609bc6a7e 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -89,6 +89,8 @@ apply_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
 	void *reloc_page;
 
 	reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK);
+	if (!reloc_page)
+		return;
 	*(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = qxl_bo_physical_address(qdev,
 											      info->src_bo,
 											      info->src_offset);
@@ -105,6 +107,8 @@ apply_surf_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
 		id = info->src_bo->surface_id;
 
 	reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo, info->dst_offset & PAGE_MASK);
+	if (!reloc_page)
+		return;
 	*(uint32_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = id;
 	qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page);
 }
@@ -161,7 +165,7 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 		return -EINVAL;
 	}
 
-	if (cmd->command_size > PAGE_SIZE - sizeof(union qxl_release_info))
+	if (cmd->command_size > 256 - sizeof(union qxl_release_info))
 		return -EINVAL;
 
 	if (!access_ok(u64_to_user_ptr(cmd->command),
@@ -188,7 +192,8 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 		 u64_to_user_ptr(cmd->command), cmd->command_size);
 
 	{
-		struct qxl_drawable *draw = fb_cmd;
+		struct qxl_drawable *draw =
+			fb_cmd + (release->release_offset & ~PAGE_MASK);
 
 		draw->mm_time = qdev->rom->mm_clock;
 	}
@@ -204,6 +209,7 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 	for (i = 0; i < cmd->relocs_num; ++i) {
 		struct drm_qxl_reloc reloc;
 		struct drm_qxl_reloc __user *u = u64_to_user_ptr(cmd->relocs);
+		size_t reloc_size;
 
 		if (copy_from_user(&reloc, u + i, sizeof(reloc))) {
 			ret = -EFAULT;
@@ -219,14 +225,29 @@ static int qxl_process_single_command(struct qxl_device *qdev,
 			goto out_free_bos;
 		}
 		reloc_info[i].type = reloc.reloc_type;
+		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
+			     sizeof(uint64_t) : sizeof(uint32_t);
 
 		if (reloc.dst_handle) {
 			ret = qxlhw_handle_to_bo(file_priv, reloc.dst_handle, release,
 						 &reloc_info[i].dst_bo);
 			if (ret)
 				goto out_free_bos;
+			if (reloc.dst_offset > reloc_info[i].dst_bo->tbo.base.size ||
+			    reloc_info[i].dst_bo->tbo.base.size - reloc.dst_offset < reloc_size ||
+			    (reloc.dst_offset & ~PAGE_MASK) > PAGE_SIZE - reloc_size) {
+				ret = -EINVAL;
+				goto out_free_bos;
+			}
 			reloc_info[i].dst_offset = reloc.dst_offset;
 		} else {
+			if (cmd->command_size < reloc_size ||
+			    reloc.dst_offset < sizeof(union qxl_release_info) ||
+			    reloc.dst_offset > sizeof(union qxl_release_info) +
+					       cmd->command_size - reloc_size) {
+				ret = -EINVAL;
+				goto out_free_bos;
+			}
 			reloc_info[i].dst_bo = cmd_bo;
 			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
 		}
@@ -323,14 +344,17 @@ int qxl_update_area_ioctl(struct drm_device *dev, void *data, struct drm_file *f
 		qxl_ttm_placement_from_domain(qobj, qobj->type);
 		ret = ttm_bo_validate(&qobj->tbo, &qobj->placement, &ctx);
 		if (unlikely(ret))
-			goto out;
+			goto out2;
 	}
 
 	ret = qxl_bo_check_id(qdev, qobj);
 	if (ret)
 		goto out2;
-	if (!qobj->surface_id)
+	if (!qobj->surface_id) {
 		DRM_ERROR("got update area for surface with no id %d\n", update_area->handle);
+		ret = -EINVAL;
+		goto out2;
+	}
 	ret = qxl_io_update_area(qdev, qobj, &area);
 
 out2:
@@ -386,12 +410,18 @@ int qxl_alloc_surf_ioctl(struct drm_device *dev, void *data, struct drm_file *fi
 	struct drm_qxl_alloc_surf *param = data;
 	int handle;
 	int ret;
-	int size, actual_stride;
+	size_t size, actual_stride;
 	struct qxl_surface surf;
 
+	if (param->stride == INT_MIN || param->stride == 0 || param->height == 0)
+		return -EINVAL;
+
 	/* work out size allocate bo with handle */
-	actual_stride = param->stride < 0 ? -param->stride : param->stride;
-	size = actual_stride * param->height + actual_stride;
+	actual_stride = param->stride < 0 ? -(size_t)param->stride : (size_t)param->stride;
+	if (check_mul_overflow(actual_stride, (size_t)param->height, &size) ||
+	    check_add_overflow(size, actual_stride, &size) ||
+	    size > INT_MAX)
+		return -EINVAL;
 
 	surf.format = param->format;
 	surf.width = param->width;
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 313f6c30cac8..d54d5b4a6f68 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -116,6 +116,8 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned long size,
 	else
 		type = ttm_bo_type_device;
 	*bo_ptr = NULL;
+	if (size == 0 || size > ULONG_MAX - PAGE_SIZE + 1)
+		return -EINVAL;
 	bo = kzalloc_obj(struct qxl_bo);
 	if (bo == NULL)
 		return -ENOMEM;
@@ -165,10 +167,8 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct iosys_map *map)
 	}
 
 	r = ttm_bo_vmap(&bo->tbo, &bo->map);
-	if (r) {
-		qxl_bo_unpin_locked(bo);
+	if (r)
 		return r;
-	}
 	bo->map_count = 1;
 
 	/* TODO: Remove kptr in favor of map everywhere. */
@@ -223,7 +223,7 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
 	return io_mapping_map_atomic_wc(map, offset + page_offset);
 fallback:
 	if (bo->kptr) {
-		rptr = bo->kptr + (page_offset * PAGE_SIZE);
+		rptr = bo->kptr + page_offset;
 		return rptr;
 	}
 
@@ -232,7 +232,7 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
 		return NULL;
 	rptr = bo_map.vaddr; /* TODO: Use mapping abstraction properly */
 
-	rptr += page_offset * PAGE_SIZE;
+	rptr += page_offset;
 	return rptr;
 }
 
@@ -395,8 +395,11 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo *bo)
 			return ret;
 
 		ret = qxl_hw_surface_alloc(qdev, bo);
-		if (ret)
+		if (ret) {
+			qxl_surface_id_dealloc(qdev, bo->surface_id);
+			bo->surface_id = 0;
 			return ret;
+		}
 	}
 	return 0;
 }