Linux virtualization list
 help / color / mirror / Atom feed
From: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
To: Dave Airlie <airlied@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	virtualization@lists.linux.dev,
	spice-devel@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Subject: [PATCH v2] drm/qxl: fix 32-bit overflow in the ALLOC_SURF size computation
Date: Sat,  8 Aug 2026 21:26:21 -0300	[thread overview]
Message-ID: <20260809002621.122832-1-qwe.aldo@gmail.com> (raw)

qxl_alloc_surf_ioctl() works out the backing size of a surface with

	actual_stride = param->stride < 0 ? -param->stride : param->stride;
	size = actual_stride * param->height + actual_stride;

where size and actual_stride are int and param->height is __u32.  Every
operand comes straight from userspace through DRM_IOCTL_QXL_ALLOC_SURF,
which is DRM_AUTH, and the expression is evaluated modulo 2^32 with no
overflow check.

The wrapped value is what reaches qxl_bo_create(), which only rounds it
up to a page.  The original width, height and stride are kept verbatim
in bo->surf and are later handed to the device by qxl_hw_surface_alloc()
together with the address of that undersized allocation, so the driver
tells the host about a surface far larger than the memory backing it.

For example stride=4096, height=1048576 gives
4096 * (1048576 + 1) = 0x1_0000_1000, which truncates to 4096: a
one-page buffer object described to the device as a 4 GiB surface.
Measured on 6.12.101 by probing mmap() lengths against the resulting
GEM object, the backing is 4096 bytes while the surface declared to the
device is 4294967296 bytes.

Two smaller problems are fixed at the same time.  Negating param->stride
is undefined for INT_MIN, and unlike QXL_ALLOC there is no rejection of
a zero-sized request.

Compute the size in u64, which cannot wrap for the u32 inputs, reject
INT_MIN and zero dimensions, and bound the result so it still fits the
int parameter of qxl_gem_object_create().

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: compute the size in u64 instead of size_t, so the (height + 1)
    addition cannot wrap on 32-bit either (reported by Sashiko AI review);
    the v1 <linux/overflow.h> include is no longer needed.

 drivers/gpu/drm/qxl/qxl_ioctl.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index cd1901d5c7c0..75abfc1db0f5 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -385,12 +385,24 @@ 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;
+	int actual_stride;
+	u64 size;
 	struct qxl_surface surf;
 
 	/* work out size allocate bo with handle */
+	if (param->stride == INT_MIN)
+		return -EINVAL;
 	actual_stride = param->stride < 0 ? -param->stride : param->stride;
-	size = actual_stride * param->height + actual_stride;
+	if (!actual_stride || !param->width || !param->height)
+		return -EINVAL;
+	/*
+	 * size = actual_stride * (height + 1), computed in u64 so it cannot
+	 * wrap on any architecture, then bounded so it still fits the int
+	 * parameter of qxl_gem_object_create().
+	 */
+	size = (u64)actual_stride * ((u64)param->height + 1);
+	if (size > INT_MAX)
+		return -EINVAL;
 
 	surf.format = param->format;
 	surf.width = param->width;
-- 
2.43.0


                 reply	other threads:[~2026-08-09  0:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260809002621.122832-1-qwe.aldo@gmail.com \
    --to=qwe.aldo@gmail.com \
    --cc=airlied@gmail.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --cc=spice-devel@lists.freedesktop.org \
    --cc=tzimmermann@suse.de \
    --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