public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
@ 2026-05-18  7:17 w15303746062
  2026-05-22  8:22 ` Thomas Zimmermann
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: w15303746062 @ 2026-05-18  7:17 UTC (permalink / raw)
  To: zack.rusin, maarten.lankhorst, mripard, tzimmermann, airlied,
	simona
  Cc: bcm-kernel-feedback-list, dri-devel, linux-kernel, stable,
	Mingyu Wang

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

When vmwgfx is configured to use VKMS for vblank simulation, it relies
on drm_calc_timestamping_constants() to calculate the frame duration
(vblank->framedur_ns).

However, Fuzzers (like Syzkaller) can submit extremely malicious
display modes through DRM_IOCTL_MODE_SETCRTC. If the user-space passes
a mode with a massive pixel clock (crtc_clock) and small resolution
(htotal/vtotal), the integer division in drm_calc_timestamping_constants()
truncates the result to 0.

Consequently, vmw_vkms_enable_vblank() blindly sets the hrtimer period
to 0. When the timer is started, it fires instantly and continuously.
Because hrtimer_forward_now() cannot advance time for a 0-period,
the overrun value skyrockets, locking the CPU in an infinite hard-IRQ
loop (vkms_vblank_simulate() -> HRTIMER_RESTART).

This completely starves the CPU, leading to massive RCU stalls and
blocking other essential tasks (like jbd2 and writeback workers)
indefinitely:

  [ C1] vkms_vblank_simulate: vblank timer overrun
  ...
  INFO: task kworker/u18:2:50 blocked for more than 143 seconds.
  Workqueue: writeback wb_workfn (flush-8:0)
  Call Trace:
   <TASK>
   __schedule+0x1044/0x5bb0
   wbt_wait+0x1c8/0x3b0
   blk_mq_submit_bio+0x29fa/0x31f0
   submit_bio_noacct+0xca7/0x1f90
   ext4_bio_write_folio+0x95a/0x1d10
   ...

  NMI backtrace for cpu 1
  Call Trace:
   <IRQ>
   vkms_vblank_simulate+0x8f/0x390
   __hrtimer_run_queues+0x1f5/0xb30
   hrtimer_interrupt+0x39a/0x880

Fix this DoS vulnerability by adding a defensive sanity check in
vmw_vkms_enable_vblank() to reject a 0-ns frame duration, allowing
DRM core to gracefully fallback/reject the mode without crashing.

Fixes: cd2eb57df1b8 ("drm/vmwgfx: Implement virtual kms")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
index 5abd7f5ad2db..b3950ae424f3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
@@ -288,6 +288,16 @@ vmw_vkms_enable_vblank(struct drm_crtc *crtc)
 
 	drm_calc_timestamping_constants(crtc, &crtc->mode);
 
+	/*
+	 * DEFENSIVE CHECK:
+	 * drm_calc_timestamping_constants() can calculate a framedur_ns
+	 * of 0 if user-space provides a malicious mode with a huge
+	 * crtc_clock and small htotal/vtotal due to integer division
+	 * truncation. Prevent hrtimer interrupt storms by refusing such modes.
+	 */
+	if (WARN_ON_ONCE(vblank->framedur_ns == 0))
+		return -EINVAL;
+
 	hrtimer_setup(&du->vkms.timer, &vmw_vkms_vblank_simulate, CLOCK_MONOTONIC,
 		      HRTIMER_MODE_REL);
 	du->vkms.period_ns = ktime_set(0, vblank->framedur_ns);
-- 
2.34.1


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

* Re: [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
  2026-05-18  7:17 [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank w15303746062
@ 2026-05-22  8:22 ` Thomas Zimmermann
  2026-05-23  2:54   ` [PATCH v2] drm/vblank: Reject 0-period timers to prevent hrtimer storm w15303746062
  2026-05-25  8:01   ` Claude review: Re: [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank Claude Code Review Bot
  2026-05-25  8:01 ` Claude review: " Claude Code Review Bot
  2026-05-25  8:01 ` Claude Code Review Bot
  2 siblings, 2 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2026-05-22  8:22 UTC (permalink / raw)
  To: w15303746062, zack.rusin, maarten.lankhorst, mripard, airlied,
	simona
  Cc: bcm-kernel-feedback-list, dri-devel, linux-kernel, stable,
	Mingyu Wang

Hi

Am 18.05.26 um 09:17 schrieb w15303746062@163.com:
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
>
> When vmwgfx is configured to use VKMS for vblank simulation, it relies
> on drm_calc_timestamping_constants() to calculate the frame duration
> (vblank->framedur_ns).
>
> However, Fuzzers (like Syzkaller) can submit extremely malicious
> display modes through DRM_IOCTL_MODE_SETCRTC. If the user-space passes
> a mode with a massive pixel clock (crtc_clock) and small resolution
> (htotal/vtotal), the integer division in drm_calc_timestamping_constants()
> truncates the result to 0.
>
> Consequently, vmw_vkms_enable_vblank() blindly sets the hrtimer period
> to 0. When the timer is started, it fires instantly and continuously.
> Because hrtimer_forward_now() cannot advance time for a 0-period,
> the overrun value skyrockets, locking the CPU in an infinite hard-IRQ
> loop (vkms_vblank_simulate() -> HRTIMER_RESTART).
>
> This completely starves the CPU, leading to massive RCU stalls and
> blocking other essential tasks (like jbd2 and writeback workers)
> indefinitely:
>
>    [ C1] vkms_vblank_simulate: vblank timer overrun
>    ...
>    INFO: task kworker/u18:2:50 blocked for more than 143 seconds.
>    Workqueue: writeback wb_workfn (flush-8:0)
>    Call Trace:
>     <TASK>
>     __schedule+0x1044/0x5bb0
>     wbt_wait+0x1c8/0x3b0
>     blk_mq_submit_bio+0x29fa/0x31f0
>     submit_bio_noacct+0xca7/0x1f90
>     ext4_bio_write_folio+0x95a/0x1d10
>     ...
>
>    NMI backtrace for cpu 1
>    Call Trace:
>     <IRQ>
>     vkms_vblank_simulate+0x8f/0x390
>     __hrtimer_run_queues+0x1f5/0xb30
>     hrtimer_interrupt+0x39a/0x880
>
> Fix this DoS vulnerability by adding a defensive sanity check in
> vmw_vkms_enable_vblank() to reject a 0-ns frame duration, allowing
> DRM core to gracefully fallback/reject the mode without crashing.
>
> Fixes: cd2eb57df1b8 ("drm/vmwgfx: Implement virtual kms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
>   drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> index 5abd7f5ad2db..b3950ae424f3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> @@ -288,6 +288,16 @@ vmw_vkms_enable_vblank(struct drm_crtc *crtc)
>   
>   	drm_calc_timestamping_constants(crtc, &crtc->mode);
>   
> +	/*
> +	 * DEFENSIVE CHECK:
> +	 * drm_calc_timestamping_constants() can calculate a framedur_ns
> +	 * of 0 if user-space provides a malicious mode with a huge
> +	 * crtc_clock and small htotal/vtotal due to integer division
> +	 * truncation. Prevent hrtimer interrupt storms by refusing such modes.
> +	 */
> +	if (WARN_ON_ONCE(vblank->framedur_ns == 0))
> +		return -EINVAL;

This code does no longer exist in the development tree (i.e., drm-misc). 
Although the new implementation might have a similar issue.

Best regards
Thomas

> +
>   	hrtimer_setup(&du->vkms.timer, &vmw_vkms_vblank_simulate, CLOCK_MONOTONIC,
>   		      HRTIMER_MODE_REL);
>   	du->vkms.period_ns = ktime_set(0, vblank->framedur_ns);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)



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

* [PATCH v2] drm/vblank: Reject 0-period timers to prevent hrtimer storm
  2026-05-22  8:22 ` Thomas Zimmermann
@ 2026-05-23  2:54   ` w15303746062
  2026-05-25  8:01   ` Claude review: Re: [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank Claude Code Review Bot
  1 sibling, 0 replies; 6+ messages in thread
From: w15303746062 @ 2026-05-23  2:54 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, simona
  Cc: zack.rusin, bcm-kernel-feedback-list, dri-devel, linux-kernel,
	stable, Mingyu Wang

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

Fuzzers like Syzkaller can submit extremely malicious display modes
through DRM_IOCTL_MODE_SETCRTC. If userspace passes a mode with a
massive pixel clock (crtc_clock) and small resolution (htotal/vtotal),
the integer division in drm_calc_timestamping_constants() truncates
the resulting frame duration (vblank->framedur_ns) to 0.

When virtual display drivers (such as vmwgfx or vkms) rely on the DRM
core's software vblank simulation, drm_crtc_vblank_start_timer() is
called. It blindly converts this 0-ns framedur_ns into a ktime interval
and starts the hrtimer. An hrtimer with a 0-period fires instantly and
continuously. Since hrtimer_forward_now() cannot advance time for a
0-period, the CPU gets locked in an infinite hard-IRQ loop, starving
the system and causing massive RCU stalls.

Fix this DoS vulnerability by adding a defensive sanity check in
drm_crtc_vblank_start_timer() to reject a 0-ns frame duration, allowing
the DRM core to gracefully reject the malicious mode.

Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
Changes in v2:
- Moved the defensive check from vmwgfx to drm_vblank.c. The timer
  logic was refactored into the DRM core, so placing the check here
  protects all drivers relying on the core software vblank timer.
- Dropped WARN_ON_ONCE() to prevent unprivileged userspace from easily
  triggering kernel panics on systems with panic_on_warn enabled.

 drivers/gpu/drm/drm_vblank.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f90fb2d13e42..b38d0b30a651 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -2241,6 +2241,16 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
 
 	drm_calc_timestamping_constants(crtc, &crtc->mode);
 
+	/*
+	 * DEFENSIVE CHECK:
+	 * drm_calc_timestamping_constants() truncates framedur_ns to 0 if
+	 * userspace provides a malicious mode with a huge crtc_clock and
+	 * small htotal/vtotal. Prevent an infinite hard-IRQ loop from a
+	 * 0-period hrtimer by rejecting such modes.
+	 */
+	if (unlikely(vblank->framedur_ns == 0))
+		return -EINVAL;
+
 	spin_lock_irqsave(&vtimer->interval_lock, flags);
 	vtimer->interval = ns_to_ktime(vblank->framedur_ns);
 	spin_unlock_irqrestore(&vtimer->interval_lock, flags);
-- 
2.34.1


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

* Claude review: drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
  2026-05-18  7:17 [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank w15303746062
  2026-05-22  8:22 ` Thomas Zimmermann
@ 2026-05-25  8:01 ` Claude Code Review Bot
  2026-05-25  8:01 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:01 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
Author: w15303746062@163.com
Patches: 3
Reviewed: 2026-05-25T18:01:51.532748

---

This is a v1→v2 iteration of a single patch fixing a real DoS vulnerability: userspace can craft a display mode with huge `crtc_clock` and tiny `htotal*vtotal`, causing `drm_calc_timestamping_constants()` to truncate `framedur_ns` to 0. This leads to an infinite hrtimer hard-IRQ loop that locks up the CPU.

The v2 correctly moves the fix from the vmwgfx driver into the DRM core (`drm_crtc_vblank_start_timer()`), which is the right location — the timer setup logic was refactored into the core, so the fix there protects all drivers using software vblank simulation, not just vmwgfx. The v2 also correctly drops `WARN_ON_ONCE()` to avoid `panic_on_warn` issues from unprivileged userspace.

The fix is a reasonable defensive bandaid. A more thorough long-term fix would reject these pathological modes earlier in the pipeline (e.g., in `atomic_check` or mode validation), but as a targeted DoS prevention this is appropriate for stable backport.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
  2026-05-18  7:17 [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank w15303746062
  2026-05-22  8:22 ` Thomas Zimmermann
  2026-05-25  8:01 ` Claude review: " Claude Code Review Bot
@ 2026-05-25  8:01 ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:01 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Superseded by v2.** This patch targets the old code path in `vmw_vkms_enable_vblank()` where the hrtimer was set up directly in the vmwgfx driver. The current tree has refactored this — `vmw_vkms_enable_vblank()` now simply calls `drm_crtc_vblank_start_timer(crtc)` (vmwgfx_vkms.c:259), so this patch no longer applies.

Even if it did apply, fixing only in vmwgfx would leave other drivers using `drm_crtc_vblank_start_timer()` (e.g., vkms) still vulnerable.

The `WARN_ON_ONCE()` usage was also problematic — it lets unprivileged userspace trigger a kernel warning, which is a panic on systems with `panic_on_warn=1`.

**Verdict:** Correctly superseded by v2.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: Re: [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank
  2026-05-22  8:22 ` Thomas Zimmermann
  2026-05-23  2:54   ` [PATCH v2] drm/vblank: Reject 0-period timers to prevent hrtimer storm w15303746062
@ 2026-05-25  8:01   ` Claude Code Review Bot
  1 sibling, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-05-25  8:01 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**This is the active patch for review.**

**Correctness:** The fix is in the right place. After `drm_calc_timestamping_constants()` stores `framedur_ns` at drm_vblank.c:673, the check at the proposed insertion point catches the zero value before it reaches:
```c
vtimer->interval = ns_to_ktime(vblank->framedur_ns);
```
and the subsequent `hrtimer_start()`. Returning `-EINVAL` prevents the timer from being armed with a zero period.

**Issues:**

1. **Missing `Fixes:` tag and `Cc: stable`.** The v1 had `Fixes: cd2eb57df1b8 ("drm/vmwgfx: Implement virtual kms")` and `Cc: stable@vger.kernel.org` in the commit message, but the v2 dropped both. The `Fixes:` tag should be updated to reference the commit that introduced `drm_crtc_vblank_start_timer()` (since that's where the bug now lives), and `Cc: stable` should be retained for backport eligibility. The `stable@` address is in the email Cc but not in the commit message trailers — maintainers and tooling look at the trailers.

2. **No diagnostic logging.** When the check triggers and returns `-EINVAL`, there's no log message. A `drm_dbg()` or `drm_err()` would help developers diagnose why vblank enablement silently failed. For example:
   ```c
   if (unlikely(vblank->framedur_ns == 0)) {
       drm_err(crtc->dev, "crtc %u: refusing 0-ns frame duration\n",
               crtc->base.id);
       return -EINVAL;
   }
   ```

3. **Comment is overly verbose and uses non-standard labeling.** The `/* DEFENSIVE CHECK: ... */` block is 6 lines of comment for 2 lines of code. Kernel style prefers terse comments. Something like:
   ```c
   /* Reject modes where integer truncation yields a 0-ns frame duration. */
   ```
   would suffice.

4. **Consider whether `drm_calc_timestamping_constants()` itself should be hardened.** That function is also called from other paths (e.g., `drm_update_vblank_count()` via `drm_crtc_vblank_helper_get_vblank_timestamp_internal`). A `framedur_ns` of 0 stored in `vblank->framedur_ns` could potentially cause division-by-zero or similar issues in other consumers. Clamping to a minimum of 1 ns in `drm_calc_timestamping_constants()` itself (or better, enforcing a sane minimum frame duration) would be more robust. However, that's arguably a separate follow-up concern and this patch is valid as a targeted fix.

5. **Timer state on error path.** If this is the first call (the `!vtimer->crtc` branch was taken), the timer data structures have been initialized but the function returns an error. On a subsequent call, the `else` branch will execute `hrtimer_try_to_cancel()` on a timer that was never started. This should be harmless since `hrtimer_setup()` initializes the timer to an inactive state, but it's worth verifying there are no side effects from the partially-initialized `vtimer` (e.g., `vtimer->crtc` is now set, so the init branch won't run again).

**Minor:** The `unlikely()` annotation is fine — this is genuinely an exceptional path.

**Verdict:** The approach is sound and fixes a real vulnerability. Needs the `Fixes:` tag updated for the correct commit, `Cc: stable` re-added to trailers, a log message on the error path, and the comment trimmed. With those addressed, this should be ready.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-05-25  8:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18  7:17 [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank w15303746062
2026-05-22  8:22 ` Thomas Zimmermann
2026-05-23  2:54   ` [PATCH v2] drm/vblank: Reject 0-period timers to prevent hrtimer storm w15303746062
2026-05-25  8:01   ` Claude review: Re: [PATCH] drm/vmwgfx: Fix hrtimer interrupt storm due to 0-period vblank Claude Code Review Bot
2026-05-25  8:01 ` Claude review: " Claude Code Review Bot
2026-05-25  8:01 ` 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