Re: [PATCH v2 1/4] drm/qxl: validate relocation dst_offset against destination BO

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch fails to prevent out-of-bounds writes for system RAM BOs (dumb buffers) due to a preexisting flaw in qxl_bo_kmap_atomic_page's fallback branch.
--

commit 816a48396f060545d332f11cc2d661437076fb2f
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

drm/qxl: validate relocation dst_offset against destination BO

This patch attempts to prevent out-of-bounds writes during relocation
processing by validating that the user-provided dst_offset falls within
the destination buffer object. It adds bounds checks for both the
command/release BO and general relocation destination BOs.

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026ceff9c..e727a35c94136 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
[ ... ]
> @@ -227,10 +231,22 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>  				goto out_free_bos;
>  			reloc_info[i].dst_offset = reloc.dst_offset;
>  		} else {
[ ... ]
>  			reloc_info[i].dst_bo = cmd_bo;
>  			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
>  		}
>  
> +		if ((u64)reloc_info[i].dst_offset + write_size >
> +		    reloc_info[i].dst_bo->tbo.base.size) {

[Severity: Critical]
This is a pre-existing issue, but does this newly added bounds check fully
prevent out-of-bounds writes for system RAM BOs (dumb buffers)?

The new bounds check in qxl_process_single_command verifies that dst_offset
is within the BO size. However, it relies on the un-multiplied byte offset.
Later, apply_reloc calls qxl_bo_kmap_atomic_page, passing
dst_offset & PAGE_MASK as the page_offset argument.

If a user creates a dumb buffer (placed in TTM_PL_SYSTEM) and passes its
handle with a large dst_offset, qxl_bo_kmap_atomic_page in
drivers/gpu/drm/qxl/qxl_object.c takes a fallback branch. That fallback
branch contains the following pointer calculation:

rptr += page_offset * PAGE_SIZE;

Because page_offset is derived from a byte offset, multiplying it by
PAGE_SIZE creates a massive out-of-bounds pointer calculation (for example,
a 100MB offset becomes 400GB). Since the QXL_EXECBUFFER ioctl is accessible
to any authenticated DRM client, an unprivileged user could use this path to
write a 64-bit physical address or 32-bit surface ID into kernel memory,
which can lead to system compromise or a kernel panic.

Should the underlying multiplier bug in qxl_bo_kmap_atomic_page be addressed
alongside this patch to ensure the vulnerability is fully closed?

> +			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,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913223000.695299-1-qwe.aldo@gmail.com?part=1