Hi Dave, Gerd, Gentle ping on this v2 -- it has been sitting for ~6 weeks. The fix promotes the backing-size computation to u64 to prevent the modulo-2^32 wrap that lets userspace create a surface whose declared size vastly exceeds its backing allocation.
Is there anything else needed, or can this go into drm-misc-fixes? Thanks, Aldo On Sat, Aug 08, 2026 09:26 PM, Aldo Ariel Panzardo <[email protected]> wrote: > 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: [email protected] > Signed-off-by: Aldo Ariel Panzardo <[email protected]> > --- > 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 > >
