Re: [PATCH] drm/qxl: use correct offset when stamping drawable mm_time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/qxl: memory corruption in command ring due to incorrect pointer arithmetic
--

commit 4eafd1ab56e831f8f5645009fd6d47df82921e0f
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

drm/qxl: use correct offset when stamping drawable mm_time

This commit attempts to fix the mm_time stamping offset by applying the
release_offset and a sizeof() offset when mapping the page pointer to the
drawable structure.

Cc: stable@vger.kernel.org

> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026ceff9c..e7594d14737ce 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
> @@ -188,7 +188,9 @@ 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 +
> +			sizeof(union qxl_release_info) +
> +			(release->release_offset & ~PAGE_MASK);

[Severity: High]
Does this pointer arithmetic shift the base pointer past the release_info
header, causing memory corruption?

In struct qxl_drawable, union qxl_release_info is the first member.
The user payload is copied to fb_cmd + sizeof(union qxl_release_info) + 
(release->release_offset & ~PAGE_MASK) exactly because the payload starts 
after the header.

If the struct pointer itself is initialized with that extra 8-byte
sizeof(union qxl_release_info) shift, won't draw->mm_time end up writing 
8 bytes past the intended location in the command ring, overwriting 
fields like the surfaces_dest array and leaving the actual mm_time
uninitialized?

>  
>  		draw->mm_time = qdev->rom->mm_clock;
>  	}

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