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

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

[Aldo Ariel Panzardo]

qxl_process_single_command() maps the page containing the current
release slot and copies the userspace command into it at the
release_offset within the page:

    fb_cmd = qxl_bo_kmap_atomic_page(qdev, cmd_bo,
                (release->release_offset & PAGE_MASK));
    copy_from_user(fb_cmd + sizeof(union qxl_release_info) +
                   (release->release_offset & ~PAGE_MASK), ...);

However, the subsequent mm_time stamp casts the page-start pointer
directly:

    struct qxl_drawable *draw = fb_cmd;
    draw->mm_time = qdev->rom->mm_clock;

This writes mm_time into the release_info header or whatever sits
at the page start rather than into the drawable that was just
copied. Apply the same offset so mm_time lands in the right place.

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026..e7594d1 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);
 
 		draw->mm_time = qdev->rom->mm_clock;
 	}
-- 
2.43.0

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

[sashiko-bot]

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

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

[Aldo Ariel Panzardo]

qxl_process_single_command() maps the page containing the current
release slot and copies the userspace command into it at the
release_offset within the page:

    fb_cmd = qxl_bo_kmap_atomic_page(qdev, cmd_bo,
                (release->release_offset & PAGE_MASK));
    copy_from_user(..., fb_cmd + sizeof(union qxl_release_info) +
                   (release->release_offset & ~PAGE_MASK), ...);

However, the subsequent mm_time stamp casts the page-start pointer
directly:

    struct qxl_drawable *draw = fb_cmd;

This writes mm_time at the page start rather than into the drawable
that was just copied. The drawable begins at the page-relative
release_offset (struct qxl_drawable embeds the release_info header
as its first member, so no additional sizeof skip is needed for the
struct pointer).

Apply the page-relative offset so mm_time lands in the right place.

Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: drop the sizeof(union qxl_release_info) term — qxl_drawable
    embeds release_info as its first member, so the struct pointer
    only needs the page-relative offset (found by Sashiko AI review).

 drivers/gpu/drm/qxl/qxl_ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 591b026..65e9bfd 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -188,7 +188,8 @@ 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 = (void *)fb_cmd +
+			(release->release_offset & ~PAGE_MASK);
 
 		draw->mm_time = qdev->rom->mm_clock;
 	}
-- 
2.43.0

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

[Aldo Ariel]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

Gentle ping — any feedback on this patch?

thanks,
Aldo

On Wed, Sep 23, 2026 12:29 PM, Aldo Ariel Panzardo <qwe.aldo@gmail.com>
wrote:

> qxl_process_single_command() maps the page containing the current
> release slot and copies the userspace command into it at the
> release_offset within the page:
>
>     fb_cmd = qxl_bo_kmap_atomic_page(qdev, cmd_bo,
>                 (release->release_offset & PAGE_MASK));
>     copy_from_user(..., fb_cmd + sizeof(union qxl_release_info) +
>                    (release->release_offset & ~PAGE_MASK), ...);
>
> However, the subsequent mm_time stamp casts the page-start pointer
> directly:
>
>     struct qxl_drawable *draw = fb_cmd;
>
> This writes mm_time at the page start rather than into the drawable
> that was just copied. The drawable begins at the page-relative
> release_offset (struct qxl_drawable embeds the release_info header
> as its first member, so no additional sizeof skip is needed for the
> struct pointer).
>
> Apply the page-relative offset so mm_time lands in the right place.
>
> Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> v2: drop the sizeof(union qxl_release_info) term — qxl_drawable
>     embeds release_info as its first member, so the struct pointer
>     only needs the page-relative offset (found by Sashiko AI review).
>
>  drivers/gpu/drm/qxl/qxl_ioctl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c
> b/drivers/gpu/drm/qxl/qxl_ioctl.c
> index 591b026..65e9bfd 100644
> --- a/drivers/gpu/drm/qxl/qxl_ioctl.c
> +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
> @@ -188,7 +188,8 @@ 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 = (void *)fb_cmd +
> +                       (release->release_offset & ~PAGE_MASK);
>
>                 draw->mm_time = qdev->rom->mm_clock;
>         }
> --
> 2.43.0
>

[-- Attachment #2: Type: text/html, Size: 2960 bytes --]