All of lore.kernel.org
 help / color / mirror / Atom feed
* [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size
@ 2026-05-14  0:07 me
  2026-05-14  4:57 ` Willy Tarreau
  0 siblings, 1 reply; 2+ messages in thread
From: me @ 2026-05-14  0:07 UTC (permalink / raw)
  To: security; +Cc: virtualization, berkantkoc

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5425 bytes --]

# QXL DRM: `apply_reloc` and `apply_surf_reloc` write to `info->dst_offset` without validating against BO size

## Summary

The QXL DRM driver's relocation processing in `drivers/gpu/drm/qxl/qxl_ioctl.c` writes 64-bit and 32-bit values to `reloc_page + (info->dst_offset & ~PAGE_MASK)` after mapping `info->dst_offset & PAGE_MASK` into kernel space via `qxl_bo_kmap_atomic_page`. The `dst_offset` is taken verbatim from the user-supplied `drm_qxl_reloc` ioctl argument and is never validated against the destination buffer object's size before the write.

An aspirational comment at the top of `apply_reloc` explicitly notes that "`dst` must be validated, i.e. whole bo on vram/surfacesram", but the validation has not been implemented since the driver was introduced in 2013.

## Impact

This is **not** a host-VM-escape primitive (the writes hit guest-controlled VRAM emulated by QEMU, not host kernel memory). It is a **guest-side memory corruption** primitive:

- A guest userspace process with `DRM_AUTH` (any logged-in user) can write attacker-chosen 64-bit/32-bit values to arbitrary offsets within the QXL VRAM aperture.
- Targets reachable within the aperture include other guest processes' QXL buffer objects, QXL release-command BOs queued for the host, and ring buffers that QEMU consumes.
- Corrupting QXL command structures that QEMU/SPICE then parses **moves the attack surface** to QEMU's QXL parser (separate codebase; historically vulnerable). This kernel bug is therefore a viable priming primitive for a guest-to-host escape chain whose terminal step lives in QEMU.

CWE-787 (Out-of-bounds Write). Realistic CVSS 3.1: `AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H` ≈ **5.5 Medium** for the kernel-side bug in isolation. If chained with a QEMU QXL parser bug: higher.

This is submitted as **hardening / defense-in-depth**, not as a self-contained RCE/VM-escape claim.

## Affected versions

Linux mainline HEAD (audited 2026-05-13). The pattern is unchanged since the QXL DRM driver merged in v3.10 (2013). Commit history is sparse on this file; the issue is structural.

## Vulnerable code

`drivers/gpu/drm/qxl/qxl_ioctl.c:73-110`:

```c
struct qxl_reloc_info {
    int type;
    struct qxl_bo *dst_bo;
    uint32_t dst_offset;          /* <-- user-controlled, copied from drm_qxl_reloc.dst_offset (__u64) */
    struct qxl_bo *src_bo;
    int src_offset;
};

/*
 * dst must be validated, i.e. whole bo on vram/surfacesram (right now all bo's
 * are on vram).
 * *(dst + dst_off) = qxl_bo_physical_address(src, src_off)
 */
static void
apply_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
{
    void *reloc_page;

    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)) =
        qxl_bo_physical_address(qdev, info->src_bo, info->src_offset);
    qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page);
}
```

`apply_surf_reloc` (Lines 98-110) has the identical pattern for a `uint32_t` write.

### How `dst_offset` reaches `apply_reloc`

The user-space struct is defined in `include/uapi/drm/qxl_drm.h`:

```c
struct drm_qxl_reloc {
    __u64 src_offset;
    __u64 dst_offset;        /* user-controlled */
    __u32 src_handle;
    __u32 dst_handle;
    __u32 reloc_type;
    __u32 pad;
};
```

`qxl_process_single_command` (`qxl_ioctl.c:141-269`) copies the user `drm_qxl_reloc` array into `reloc_info`, narrows `dst_offset` from `__u64` to `uint32_t` (silent truncation -- still gives the attacker the full 32-bit / 4 GB range), and then calls `apply_reloc`/`apply_surf_reloc`. No bounds check against `dst_bo->tbo.base.size` exists anywhere in the call chain. `qxl_release_reserve_list` only handles TTM placement validation.

### Mapping behaviour

`qxl_bo_kmap_atomic_page` (`drivers/gpu/drm/qxl/qxl_object.c`) computes `offset = (bo->tbo.resource->start << PAGE_SHIFT) + page_offset` and maps via `io_mapping_map_atomic_wc` on the QXL device's full VRAM aperture (`vram_mapping`). The `BUG_ON(offset >= mapping->size)` inside `io_mapping_map_atomic_wc` (see `include/linux/io-mapping.h`) only fires if the offset exceeds the *entire* aperture, not the specific BO. Writes within the aperture but outside the BO succeed and land on whatever data occupies those pages.

## Proposed fix

Add the validation the comment promised, in the per-reloc-info-fill loop in `qxl_process_single_command` (around line 228-244):

```c
+   if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
+       (reloc_info[i].dst_offset + sizeof(uint64_t) > reloc_info[i].dst_bo->tbo.base.size ||
+        reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t))) {
+       ret = -EINVAL;
+       goto out_free_bos;
+   }
+   if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
+       (reloc_info[i].dst_offset + sizeof(uint32_t) > reloc_info[i].dst_bo->tbo.base.size ||
+        reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t))) {
+       ret = -EINVAL;
+       goto out_free_bos;
+   }
```

Equivalent to checking `dst_offset` plus the write size fits inside the resolved BO. Also enforce a reasonable upper bound on `cmd->relocs_num` to prevent allocation DoS.

## Discovery

Static analysis of QXL DRM ioctl handlers.

## Disclosure

Per Linux kernel security policy, report to `security@kernel.org` and the QXL maintainers (Gerd Hoffmann, virtualization@lists.linux.dev).

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size
  2026-05-14  0:07 [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size me
@ 2026-05-14  4:57 ` Willy Tarreau
  0 siblings, 0 replies; 2+ messages in thread
From: Willy Tarreau @ 2026-05-14  4:57 UTC (permalink / raw)
  To: me; +Cc: security, virtualization, berkantkoc

Hello,

On Thu, May 14, 2026 at 02:07:47AM +0200, me@berkoc.com wrote:
> # QXL DRM: `apply_reloc` and `apply_surf_reloc` write to `info->dst_offset` without validating against BO size
> 
> ## Summary
> 
> The QXL DRM driver's relocation processing in `drivers/gpu/drm/qxl/qxl_ioctl.c` writes 64-bit and 32-bit values to `reloc_page + (info->dst_offset & ~PAGE_MASK)` after mapping `info->dst_offset & PAGE_MASK` into kernel space via `qxl_bo_kmap_atomic_page`. The `dst_offset` is taken verbatim from the user-supplied `drm_qxl_reloc` ioctl argument and is never validated against the destination buffer object's size before the write.
> 
> An aspirational comment at the top of `apply_reloc` explicitly notes that "`dst` must be validated, i.e. whole bo on vram/surfacesram", but the validation has not been implemented since the driver was introduced in 2013.
> 
> ## Impact
> 
> This is **not** a host-VM-escape primitive (the writes hit guest-controlled VRAM emulated by QEMU, not host kernel memory). It is a **guest-side memory corruption** primitive:
> 
> - A guest userspace process with `DRM_AUTH` (any logged-in user) can write attacker-chosen 64-bit/32-bit values to arbitrary offsets within the QXL VRAM aperture.
> - Targets reachable within the aperture include other guest processes' QXL buffer objects, QXL release-command BOs queued for the host, and ring buffers that QEMU consumes.
> - Corrupting QXL command structures that QEMU/SPICE then parses **moves the attack surface** to QEMU's QXL parser (separate codebase; historically vulnerable). This kernel bug is therefore a viable priming primitive for a guest-to-host escape chain whose terminal step lives in QEMU.
> 
> CWE-787 (Out-of-bounds Write). Realistic CVSS 3.1: `AV:L/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:H` ≈ **5.5 Medium** for the kernel-side bug in isolation. If chained with a QEMU QXL parser bug: higher.
> 
> This is submitted as **hardening / defense-in-depth**, not as a self-contained RCE/VM-escape claim.
> 
> ## Affected versions
> 
> Linux mainline HEAD (audited 2026-05-13). The pattern is unchanged since the QXL DRM driver merged in v3.10 (2013). Commit history is sparse on this file; the issue is structural.
> 
> ## Vulnerable code
> 
> `drivers/gpu/drm/qxl/qxl_ioctl.c:73-110`:
> 
> ```c
> struct qxl_reloc_info {
>     int type;
>     struct qxl_bo *dst_bo;
>     uint32_t dst_offset;          /* <-- user-controlled, copied from drm_qxl_reloc.dst_offset (__u64) */
>     struct qxl_bo *src_bo;
>     int src_offset;
> };
> 
> /*
>  * dst must be validated, i.e. whole bo on vram/surfacesram (right now all bo's
>  * are on vram).
>  * *(dst + dst_off) = qxl_bo_physical_address(src, src_off)
>  */
> static void
> apply_reloc(struct qxl_device *qdev, struct qxl_reloc_info *info)
> {
>     void *reloc_page;
> 
>     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)) =
>         qxl_bo_physical_address(qdev, info->src_bo, info->src_offset);
>     qxl_bo_kunmap_atomic_page(qdev, info->dst_bo, reloc_page);
> }
> ```
> 
> `apply_surf_reloc` (Lines 98-110) has the identical pattern for a `uint32_t` write.
> 
> ### How `dst_offset` reaches `apply_reloc`
> 
> The user-space struct is defined in `include/uapi/drm/qxl_drm.h`:
> 
> ```c
> struct drm_qxl_reloc {
>     __u64 src_offset;
>     __u64 dst_offset;        /* user-controlled */
>     __u32 src_handle;
>     __u32 dst_handle;
>     __u32 reloc_type;
>     __u32 pad;
> };
> ```
> 
> `qxl_process_single_command` (`qxl_ioctl.c:141-269`) copies the user `drm_qxl_reloc` array into `reloc_info`, narrows `dst_offset` from `__u64` to `uint32_t` (silent truncation -- still gives the attacker the full 32-bit / 4 GB range), and then calls `apply_reloc`/`apply_surf_reloc`. No bounds check against `dst_bo->tbo.base.size` exists anywhere in the call chain. `qxl_release_reserve_list` only handles TTM placement validation.
> 
> ### Mapping behaviour
> 
> `qxl_bo_kmap_atomic_page` (`drivers/gpu/drm/qxl/qxl_object.c`) computes `offset = (bo->tbo.resource->start << PAGE_SHIFT) + page_offset` and maps via `io_mapping_map_atomic_wc` on the QXL device's full VRAM aperture (`vram_mapping`). The `BUG_ON(offset >= mapping->size)` inside `io_mapping_map_atomic_wc` (see `include/linux/io-mapping.h`) only fires if the offset exceeds the *entire* aperture, not the specific BO. Writes within the aperture but outside the BO succeed and land on whatever data occupies those pages.
> 
> ## Proposed fix
> 
> Add the validation the comment promised, in the per-reloc-info-fill loop in `qxl_process_single_command` (around line 228-244):
> 
> ```c
> +   if (reloc_info[i].type == QXL_RELOC_TYPE_BO &&
> +       (reloc_info[i].dst_offset + sizeof(uint64_t) > reloc_info[i].dst_bo->tbo.base.size ||
> +        reloc_info[i].dst_offset > U32_MAX - sizeof(uint64_t))) {
> +       ret = -EINVAL;
> +       goto out_free_bos;
> +   }
> +   if (reloc_info[i].type == QXL_RELOC_TYPE_SURF &&
> +       (reloc_info[i].dst_offset + sizeof(uint32_t) > reloc_info[i].dst_bo->tbo.base.size ||
> +        reloc_info[i].dst_offset > U32_MAX - sizeof(uint32_t))) {
> +       ret = -EINVAL;
> +       goto out_free_bos;
> +   }
> ```

Please turn this fix into a regular patch that can be applied. If it's
accepted by maintainers, it will save them some time and will have you
credited for finding and fixing this bug.

> Equivalent to checking `dst_offset` plus the write size fits inside the resolved BO. Also enforce a reasonable upper bound on `cmd->relocs_num` to prevent allocation DoS.
> 
> ## Discovery
> 
> Static analysis of QXL DRM ioctl handlers.
> 
> ## Disclosure
> 
> Per Linux kernel security policy, report to `security@kernel.org` and the QXL maintainers (Gerd Hoffmann, virtualization@lists.linux.dev).

Thanks for your report. However, this report is already public via the
virtualization list, so no need to involve security@, and it can be
dropped from future responses:

   https://lore.kernel.org/virtualization/36acd33982bfdce04090e17294596ff8@berkoc.com/T/#u

Willy

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-14  4:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14  0:07 [Hardening] drm/qxl: apply_reloc/apply_surf_reloc write dst_offset without validating against BO size me
2026-05-14  4:57 ` Willy Tarreau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.