Re: [PATCH v2] drm/qxl: validate relocation destination offsets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] qxl: unaligned dst_offset causes page-boundary cross and alignment faults
--

commit 214f69f63b016ba44fb4af3f2bea1273b9aa972b
Author: Zhou Qiuyue <n0ps1edzz@gmail.com>

drm/qxl: validate relocation destination offsets

This patch validates the user-supplied relocation offset against the
destination object's size to prevent out-of-bounds kernel writes. It also
widens data types to prevent truncation and sign-extension vulnerabilities.

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026ceff9c..ae32295fd5e84 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
[ ... ]
> @@ -231,6 +232,23 @@ static int qxl_process_single_command(struct qxl_device *qdev,
>  			reloc_info[i].dst_offset = reloc.dst_offset + release->release_offset;
>  		}
>  
> +		/*
> +		 * The relocation is applied with a plain write into the memory
> +		 * window of the destination object, so the user supplied offset
> +		 * must be validated against the object size.  Otherwise userspace
> +		 * can write anywhere in the mapping window set up by
> +		 * qxl_bo_kmap_atomic_page(), i.e. outside of the object.
> +		 */
> +		reloc_size = (reloc.reloc_type == QXL_RELOC_TYPE_BO) ?
> +			     sizeof(uint64_t) : sizeof(uint32_t);
> +		if (reloc_info[i].dst_offset > reloc_info[i].dst_bo->tbo.base.size ||
> +		    reloc_size > reloc_info[i].dst_bo->tbo.base.size -
> +				 reloc_info[i].dst_offset) {

[Severity: High]
Does this bounds check ensure that dst_offset is properly aligned to
reloc_size?

If userspace passes an unaligned dst_offset, could the pointer write in
apply_reloc() cross a page boundary?

In qxl_ioctl.c:apply_reloc():
    reloc_page = qxl_bo_kmap_atomic_page(qdev, info->dst_bo,
                                         info->dst_offset & PAGE_MASK);
    *(uint64_t *)(reloc_page + (info->dst_offset & ~PAGE_MASK)) = ...;

If (dst_offset & ~PAGE_MASK) > PAGE_SIZE - 8, the 8-byte write could cross
into an unmapped or adjacent page, potentially causing a fault when using
CONFIG_HAVE_ATOMIC_IOMAP.

Additionally, could the unaligned cast to (uint64_t *) trigger unaligned access
exceptions on architectures with strict alignment requirements?

> +			DRM_DEBUG("reloc dst offset out of bounds\n");
> +			ret = -EINVAL;
> +			goto out_free_bos;
> +		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6acafc03.6b76d992.279a8.b343@mx.google.com?part=1