public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [REPORT] drm/vmwgfx: vmw_cmd_draw header.size lower-bound missing - guest-local OOB-read/write
@ 2026-05-17 13:00 Berkant Koc
  2026-05-17 13:37 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Berkant Koc @ 2026-05-17 13:00 UTC (permalink / raw)
  To: security
  Cc: Zack Rusin, bcm-kernel-feedback-list, dri-devel, Daniel Vetter,
	David Airlie, Thomas Zimmermann

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi all,

Following the drm/qxl apply_reloc report from 14 May 2026
(prior thread: https://lore.kernel.org/virtualization/36acd33982bfdce04090e17294596ff8@berkoc.com/T/),
I audited the DRM cousin-drivers for the same bug-class. vmwgfx exposes a
stronger instance.

Primary site: `vmw_cmd_draw` in
`drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:1558-1607`. The verifier does

    maxnum = (header->size - sizeof(cmd->body)) / sizeof(*decl);
    if (cmd->body.numVertexDecls > maxnum) return -EINVAL;

Commit `32b415a9dc2c` ("drm/vmwgfx: Validate command header size against
SVGA_CMD_MAX_DATASIZE", Oct 2025) added the upper bound but not the
lower bound. With `header->size < sizeof(cmd->body)` the size_t
subtraction wraps, `maxnum` becomes ~SIZE_MAX, and the loop walks
attacker-chosen `numVertexDecls` entries past the cmd-buffer. Each
iteration is a 4-byte OOB-read via `vmw_cmd_res_check`; on a handle
collision `vmw_resource_relocation_add` records the OOB address as
`rel->offset` (29-bit), and `vmw_resource_relocations_apply` later does
a 32-bit kernel write at `cb + rel->offset` within a 512 MiB window.

Exploitability split:
  - OOB-read at first iteration is deterministic.
  - OOB-write requires `id_loc` to resolve to a valid surface handle;
    surface-spray makes this practical but the write side is probabilistic.

Two cousin sites share the same root cause (details in BUG.md):
  - `vmw_cmd_dma` (line 1495): pointer underflow -> 4-byte OOB-read of
    `suffix->suffixSize`.
  - `vmw_cmd_shader_define` (line 1898): `size = header.size - sizeof(body)`
    wraps and reaches `vmw_compat_shader_add`; ENOMEM-bounded DoS.

Reachability: DRM_VMW_EXECBUF, DRM_RENDER_ALLOW. Any `/dev/dri/renderD128`
user (default `video` group) on a VMware or KVM SVGA-II guest. No DRM_AUTH,
no DRM_MASTER, no root.

CVSS 3.1: `CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H` -> 7.8 High
(deterministic OOB-read, probabilistic 32-bit kernel write within 512 MiB;
guest-local LPE, no host escape claimed).

Suggested fix: per-site lower-bound check on `header->size` before the
subtraction, or the safe pattern using `check_mul_overflow` /
`check_add_overflow`. A draft patch is sent as the follow-up RFC in
this thread.

Attachments: BUG.md (full trace) and repro.c (KASAN-trigger PoC, no
exploitation).

Happy to follow your standard timeline per Documentation/process/security-bugs.rst
(typically <=7 days post-fix-merge).

Berkant Koc <me@berkoc.com>
GPG: 0C58 8DFD 7620 4987 2842 13EA 0AC5 29C4 1F8A A5D6
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQQMWI39diBJhyhCE+oKxSnEH4ql1gUCagm/bwAKCRAKxSnEH4ql
1i1PAQCXhrf6CdxpwCifgpjYZJdzspd9dckht9le4AeUJpKRpwD/csbOYVl+oMDY
mekPtFhNq+TujSRVzvRZ0MKzOawzAwU=
=7oh3
-----END PGP SIGNATURE-----

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

* [PATCH] drm/vmwgfx: validate execbuf header.size lower bound
  2026-05-17 13:37 ` Greg KH
@ 2026-05-17 13:05   ` Berkant Koc
  2026-05-17 23:23     ` Zack Rusin
  2026-05-18  6:07     ` Claude review: " Claude Code Review Bot
  0 siblings, 2 replies; 6+ messages in thread
From: Berkant Koc @ 2026-05-17 13:05 UTC (permalink / raw)
  To: Zack Rusin, bcm-kernel-feedback-list, dri-devel
  Cc: security, Daniel Vetter, David Airlie, Thomas Zimmermann, stable

Commit 32b415a9dc2c ("drm/vmwgfx: Validate command header size against
SVGA_CMD_MAX_DATASIZE") added an upper bound on the user-supplied
SVGA3dCmdHeader.size field but no matching lower bound. When
header->size is smaller than sizeof(cmd->body), the size_t subtraction
in expressions like

  maxnum = (header->size - sizeof(cmd->body)) / sizeof(*decl);

underflows. The subsequent bound check

  if (cmd->body.numVertexDecls > maxnum) return -EINVAL;

is bypassed because maxnum is ~SIZE_MAX, and the loop walks
attacker-chosen entries past the command buffer.

In vmw_cmd_draw this leads to a 4-byte OOB-read per iteration via
vmw_cmd_res_check(&decl[i].array.surfaceId, ...); on a surface-handle
collision, vmw_resource_relocation_add records the OOB address as
rel->offset (29-bit bitfield), and vmw_resource_relocations_apply
later performs a 32-bit kernel write at cb + rel->offset.

The same root cause is present in vmw_cmd_dma (suffix pointer-arith
underflow leading to OOB-read of suffix->suffixSize) and
vmw_cmd_shader_define (size_t wraparound passed to
vmw_compat_shader_add).

Reachable via DRM_VMW_EXECBUF (DRM_RENDER_ALLOW). Reject undersized
headers at all three sites before the subtraction.

Cc: stable@vger.kernel.org # v6.18+
Fixes: 32b415a9dc2c ("drm/vmwgfx: Validate command header size against SVGA_CMD_MAX_DATASIZE")
Signed-off-by: Berkant Koc <me@berkoc.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index e1f18020170a..6f9c7d61cc66 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -1506,6 +1506,12 @@ static int vmw_cmd_dma(struct vmw_private *dev_priv,
 	bool dirty;
 
 	cmd = container_of(header, typeof(*cmd), header);
+
+	if (unlikely(header->size < sizeof(cmd->body) + sizeof(*suffix))) {
+		VMW_DEBUG_USER("DMA cmd header.size too small.\n");
+		return -EINVAL;
+	}
+
 	suffix = (SVGA3dCmdSurfaceDMASuffix *)((unsigned long) &cmd->body +
 					       header->size - sizeof(*suffix));
 
@@ -1572,6 +1578,12 @@ static int vmw_cmd_draw(struct vmw_private *dev_priv,
 		return ret;
 
 	cmd = container_of(header, typeof(*cmd), header);
+
+	if (unlikely(header->size < sizeof(cmd->body))) {
+		VMW_DEBUG_USER("Draw cmd header.size smaller than body.\n");
+		return -EINVAL;
+	}
+
 	maxnum = (header->size - sizeof(cmd->body)) / sizeof(*decl);
 
 	if (unlikely(cmd->body.numVertexDecls > maxnum)) {
@@ -1915,6 +1927,11 @@ static int vmw_cmd_shader_define(struct vmw_private *dev_priv,
 	if (unlikely(!dev_priv->has_mob))
 		return 0;
 
+	if (unlikely(cmd->header.size < sizeof(cmd->body))) {
+		VMW_DEBUG_USER("Shader define cmd header.size smaller than body.\n");
+		return -EINVAL;
+	}
+
 	size = cmd->header.size - sizeof(cmd->body);
 	ret = vmw_compat_shader_add(dev_priv, vmw_context_res_man(ctx),
 				    cmd->body.shid, cmd + 1, cmd->body.type,
-- 
2.47.3


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

* Re: [REPORT] drm/vmwgfx: vmw_cmd_draw header.size lower-bound missing - guest-local OOB-read/write
  2026-05-17 13:00 [REPORT] drm/vmwgfx: vmw_cmd_draw header.size lower-bound missing - guest-local OOB-read/write Berkant Koc
@ 2026-05-17 13:37 ` Greg KH
  2026-05-17 13:05   ` [PATCH] drm/vmwgfx: validate execbuf header.size lower bound Berkant Koc
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2026-05-17 13:37 UTC (permalink / raw)
  To: Berkant Koc
  Cc: security, Zack Rusin, bcm-kernel-feedback-list, dri-devel,
	Daniel Vetter, David Airlie, Thomas Zimmermann

On Sun, May 17, 2026 at 03:00:00PM +0200, Berkant Koc wrote:
> Happy to follow your standard timeline per Documentation/process/security-bugs.rst
> (typically <=7 days post-fix-merge).

You sent this to a public list, there is no "timeline" needed.

Just submit a fix for this now please.

thanks,

greg k-h

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

* Re: [PATCH] drm/vmwgfx: validate execbuf header.size lower bound
  2026-05-17 13:05   ` [PATCH] drm/vmwgfx: validate execbuf header.size lower bound Berkant Koc
@ 2026-05-17 23:23     ` Zack Rusin
  2026-05-18  0:56       ` Berkant Koc
  2026-05-18  6:07     ` Claude review: " Claude Code Review Bot
  1 sibling, 1 reply; 6+ messages in thread
From: Zack Rusin @ 2026-05-17 23:23 UTC (permalink / raw)
  To: Berkant Koc
  Cc: bcm-kernel-feedback-list, dri-devel, Daniel Vetter, David Airlie,
	Thomas Zimmermann, stable

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

On Sun, May 17, 2026 at 10:07 AM Berkant Koc <me@berkoc.com> wrote:
>
> Commit 32b415a9dc2c ("drm/vmwgfx: Validate command header size against
> SVGA_CMD_MAX_DATASIZE") added an upper bound on the user-supplied
> SVGA3dCmdHeader.size field but no matching lower bound. When
> header->size is smaller than sizeof(cmd->body), the size_t subtraction
> in expressions like
>
>   maxnum = (header->size - sizeof(cmd->body)) / sizeof(*decl);
>
> underflows. The subsequent bound check
>
>   if (cmd->body.numVertexDecls > maxnum) return -EINVAL;
>
> is bypassed because maxnum is ~SIZE_MAX, and the loop walks
> attacker-chosen entries past the command buffer.
>
> In vmw_cmd_draw this leads to a 4-byte OOB-read per iteration via
> vmw_cmd_res_check(&decl[i].array.surfaceId, ...); on a surface-handle
> collision, vmw_resource_relocation_add records the OOB address as
> rel->offset (29-bit bitfield), and vmw_resource_relocations_apply
> later performs a 32-bit kernel write at cb + rel->offset.
>
> The same root cause is present in vmw_cmd_dma (suffix pointer-arith
> underflow leading to OOB-read of suffix->suffixSize) and
> vmw_cmd_shader_define (size_t wraparound passed to
> vmw_compat_shader_add).
>
> Reachable via DRM_VMW_EXECBUF (DRM_RENDER_ALLOW). Reject undersized
> headers at all three sites before the subtraction.
>
> Cc: stable@vger.kernel.org # v6.18+
> Fixes: 32b415a9dc2c ("drm/vmwgfx: Validate command header size against SVGA_CMD_MAX_DATASIZE")
> Signed-off-by: Berkant Koc <me@berkoc.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> index e1f18020170a..6f9c7d61cc66 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> @@ -1506,6 +1506,12 @@ static int vmw_cmd_dma(struct vmw_private *dev_priv,
>         bool dirty;
>
>         cmd = container_of(header, typeof(*cmd), header);
> +
> +       if (unlikely(header->size < sizeof(cmd->body) + sizeof(*suffix))) {
> +               VMW_DEBUG_USER("DMA cmd header.size too small.\n");
> +               return -EINVAL;
> +       }
> +
>         suffix = (SVGA3dCmdSurfaceDMASuffix *)((unsigned long) &cmd->body +
>                                                header->size - sizeof(*suffix));
>
> @@ -1572,6 +1578,12 @@ static int vmw_cmd_draw(struct vmw_private *dev_priv,
>                 return ret;
>
>         cmd = container_of(header, typeof(*cmd), header);
> +
> +       if (unlikely(header->size < sizeof(cmd->body))) {
> +               VMW_DEBUG_USER("Draw cmd header.size smaller than body.\n");
> +               return -EINVAL;
> +       }
> +
>         maxnum = (header->size - sizeof(cmd->body)) / sizeof(*decl);
>
>         if (unlikely(cmd->body.numVertexDecls > maxnum)) {
> @@ -1915,6 +1927,11 @@ static int vmw_cmd_shader_define(struct vmw_private *dev_priv,
>         if (unlikely(!dev_priv->has_mob))
>                 return 0;
>
> +       if (unlikely(cmd->header.size < sizeof(cmd->body))) {
> +               VMW_DEBUG_USER("Shader define cmd header.size smaller than body.\n");
> +               return -EINVAL;
> +       }
> +
>         size = cmd->header.size - sizeof(cmd->body);
>         ret = vmw_compat_shader_add(dev_priv, vmw_context_res_man(ctx),
>                                     cmd->body.shid, cmd + 1, cmd->body.type,
> --
> 2.47.3
>

I think you might have forgotten to disclose the tool/llm you've used
for this. Using Claude Opus 4.7 I've found the same issues and the
series fixing those bugs is available on dri-devel at
https://patchwork.freedesktop.org/series/166024/ . If you've used a
different llm I'd love to know which one. And if you have a completely
different tool for detecting those issues that'd be great to know as
well.

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

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

* Re: [PATCH] drm/vmwgfx: validate execbuf header.size lower bound
  2026-05-17 23:23     ` Zack Rusin
@ 2026-05-18  0:56       ` Berkant Koc
  0 siblings, 0 replies; 6+ messages in thread
From: Berkant Koc @ 2026-05-18  0:56 UTC (permalink / raw)
  To: Zack Rusin
  Cc: bcm-kernel-feedback-list, dri-devel, Daniel Vetter, David Airlie,
	Thomas Zimmermann, stable

Thanks for the catch, you're right, I should have flagged the tooling
in the patch body. And cross-pollination with your own series on
patchwork is a good outcome.

Tooling: berkoc-pipeline, a custom RAG framework on Claude Opus 4.7
(Anthropic CVP cohort, May 2026). Full agentic stack: multi-tool
execution (filesystem, web fetch, code execution), parallel subagent
orchestration with adaptive task decomposition, extended-thinking
integration, retrieval-augmented context over a file-based semantic
knowledge base, MCP-style integration patterns. 7-step pre-disclosure
validation gate, manual verification on every finding before submit.

v2 of this patch will include the formal trailer:
Assisted-by: Claude:claude-opus-4-7 berkoc-pipeline

Happy to send v2 with the trailer formalised, or keep the methodology
disclosure to follow-up comments if you prefer.

Berkant

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

* Claude review: drm/vmwgfx: validate execbuf header.size lower bound
  2026-05-17 13:05   ` [PATCH] drm/vmwgfx: validate execbuf header.size lower bound Berkant Koc
  2026-05-17 23:23     ` Zack Rusin
@ 2026-05-18  6:07     ` Claude Code Review Bot
  1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:07 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/vmwgfx: validate execbuf header.size lower bound
Author: Berkant Koc <me@berkoc.com>
Patches: 3
Reviewed: 2026-05-18T16:07:05.252863

---

This is a single-patch security fix for integer underflow vulnerabilities in the vmwgfx execbuf command validation. The analysis in the commit message is accurate and well-written — the missing lower-bound check on `header->size` allows `size_t` underflow in three separate command handlers, all reachable from unprivileged userspace via `DRM_VMW_EXECBUF` (which has `DRM_RENDER_ALLOW`).

The patch is **correct and necessary**, but it has one gap: the second `maxnum` calculation in `vmw_cmd_draw` (line 1591-1592) can still underflow even after the patch's check passes.

**Severity**: High. The commit message accurately describes a path from OOB-read to kernel write via `vmw_resource_relocations_apply`. This is reachable from render nodes.

**Recommendation**: The patch should be accepted with one fix — the `vmw_cmd_draw` check needs to be tightened, or a second check is needed before the second `maxnum` calculation.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-18  6:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 13:00 [REPORT] drm/vmwgfx: vmw_cmd_draw header.size lower-bound missing - guest-local OOB-read/write Berkant Koc
2026-05-17 13:37 ` Greg KH
2026-05-17 13:05   ` [PATCH] drm/vmwgfx: validate execbuf header.size lower bound Berkant Koc
2026-05-17 23:23     ` Zack Rusin
2026-05-18  0:56       ` Berkant Koc
2026-05-18  6:07     ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox