[PATCH] drm/vkms: sanitize display mode to prevent hrtimer livelock

[PATCH] drm/vkms: sanitize display mode to prevent hrtimer livelock

[w15303746062]

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

SyzKaller reported an RCU stall caused by a livelock in
vkms_crtc_handle_vblank_timeout.

The root cause is that vkms derives its vblank timer period directly
from the display mode, but does not validate the mode parameters in
its atomic_check. A fuzzer or malicious user can pass a mode with
either an explicit vrefresh > 1000 Hz, or a combination of huge
crtc_clock and tiny htotal/vtotal that causes the computed vrefresh
to overflow or result in a frame duration close to zero.

When the period approaches zero, the hrtimer fires faster than it can
be handled, leading to an interrupt storm that deadlocks the CPU.

Fix this by rejecting modes in atomic_check where:
 - the explicit vrefresh exceeds 1000 Hz, or
 - the implied frame duration (derived from crtc_clock/htotal/vtotal)
   is less than 1 ms (equivalent to >1000 Hz).

This also protects against integer overflow in drm_mode_vrefresh().

SyzKaller logs snippet of the failure:
[  392.807933][    C2] vkms_vblank_simulate: vblank timer overrun
...
[  592.384301][    C3] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[  592.438915][    C0] RIP: 0010:native_queued_spin_lock_slowpath+0x23e/0x9c0
[  592.440560][    C0] Call Trace:
[  592.440570][    C0]  <IRQ>
[  592.448399][    C0]  drm_handle_vblank+0x132/0xc70
[  592.449106][    C0]  __hrtimer_run_queues+0x1f5/0xb30

Fixes: 02e2681ffe1a ("drm/vkms: Convert to DRM's vblank timer")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 35ddc553a5e6..20b97dd0cc5f 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -116,9 +116,31 @@ static int vkms_crtc_atomic_check(struct drm_crtc *crtc,
 	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
 									  crtc);
 	struct vkms_crtc_state *vkms_state = to_vkms_crtc_state(crtc_state);
+	struct drm_display_mode *mode = &crtc_state->adjusted_mode;
 	struct drm_plane *plane;
 	struct drm_plane_state *plane_state;
 	int i = 0, ret;
+	int vrefresh;
+	u64 frame_ns;
+
+	/*
+	 * Reject modes that would cause an hrtimer storm.
+	 * A virtual display cannot meaningfully refresh at >1000 Hz,
+	 * and an extremely high vrefresh (or a crafted combination of
+	 * crtc_clock/htotal/vtotal that overflows the vrefresh computation)
+	 * would produce a frame duration close to zero, locking the CPU
+	 * in an interrupt loop.
+	 */
+	if (crtc_state->enable) {
+		vrefresh = drm_mode_vrefresh(mode);
+		if (vrefresh > 1000)
+			return -EINVAL;
+		if (!mode->crtc_clock || !mode->htotal || !mode->vtotal)
+			return -EINVAL;
+		frame_ns = div_u64((u64)mode->htotal * mode->vtotal * 1000000ULL, mode->crtc_clock);
+		if (frame_ns < 1000000) /* <1 ms => refresh >1000 Hz */
+			return -EINVAL;
+	}
 
 	if (vkms_state->active_planes)
 		return 0;
-- 
2.34.1

Claude review: drm/vkms: sanitize display mode to prevent hrtimer livelock

[Claude Code Review Bot]

Overall Series Review

Subject: drm/vkms: sanitize display mode to prevent hrtimer livelock
Author: w15303746062@163.com
Patches: 1
Reviewed: 2026-05-27T13:52:39.566549

---

This is a single patch addressing a real syzbot-reported bug: a crafted display mode with an extremely high refresh rate can cause the vkms hrtimer to fire so rapidly it livelocks the CPU. The fix approach — rejecting pathological modes in `atomic_check` — is reasonable and addresses the right layer (the driver that accepts arbitrary modes). However, the patch has several correctness issues that would prevent it from fully closing the vulnerability.

**Key concerns:**

1. **Wrong mode fields validated.** The vblank timer period is derived from `crtc_clock`/`crtc_htotal`/`crtc_vtotal` (in `drm_calc_timestamping_constants()`), but the patch's manual frame duration check uses `htotal`/`vtotal` (not the `crtc_` variants). While these are often the same after mode adjustment, they can differ — and the `adjusted_mode` should have the `crtc_` prefixed fields validated to match what the timer actually uses.

2. **`drm_mode_vrefresh()` uses different fields than the timer.** `drm_mode_vrefresh()` uses `mode->clock` / `mode->htotal` / `mode->vtotal`, while the timer uses `mode->crtc_clock` / `mode->crtc_htotal` / `mode->crtc_vtotal`. A mode could pass the `drm_mode_vrefresh()` check but still produce a near-zero `framedur_ns`.

3. **Ordering issue with `vrefresh` check and zero-field check.** If `htotal` or `vtotal` is zero, `drm_mode_vrefresh()` returns 0, which passes the `> 1000` check. The subsequent zero-field check catches `htotal == 0` or `vtotal == 0`, but calling `drm_mode_vrefresh()` first with potentially degenerate fields is unnecessary — the zero check should come first.

4. **Interlace modes are not accounted for.** `drm_calc_timestamping_constants()` halves `framedur_ns` for interlaced modes (`DRM_MODE_FLAG_INTERLACE`), but the patch's manual frame duration calculation does not. An interlaced mode at 600 Hz effective would produce `framedur_ns` below 1 ms after halving, but the patch would compute ~1.67 ms and let it through.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/vkms: sanitize display mode to prevent hrtimer livelock

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- The commit message is well-written, clearly describes the bug, and includes syzbot logs.
- The `Fixes:` tag and `Cc: stable` are appropriate.
- The overall approach (reject in `atomic_check`) is the right place for this validation.
- The 1000 Hz threshold is reasonable for a virtual display.

**Issue 1: Wrong mode fields for frame duration calculation (bug)**

The vblank timer period is computed from `crtc_clock`/`crtc_htotal`/`crtc_vtotal` in `drm_calc_timestamping_constants()` (`drm_vblank.c:652-660`), but the patch validates `htotal`/`vtotal`:

```c
if (!mode->crtc_clock || !mode->htotal || !mode->vtotal)
    return -EINVAL;
frame_ns = div_u64((u64)mode->htotal * mode->vtotal * 1000000ULL, mode->crtc_clock);
```

This should use `mode->crtc_htotal` and `mode->crtc_vtotal` to match the actual timer computation. While the `adjusted_mode` often has `crtc_` fields matching non-prefixed fields, they can differ after mode fixup, and the `crtc_` fields are what determine the actual timer period.

**Issue 2: `drm_mode_vrefresh()` uses `clock`, not `crtc_clock` (bug)**

`drm_mode_vrefresh()` computes refresh from `mode->clock` / `mode->htotal` / `mode->vtotal`, but the timer period depends on `mode->crtc_clock` / `mode->crtc_htotal` / `mode->crtc_vtotal`. A mode could have a sane `clock`/`htotal`/`vtotal` but a crafted `crtc_clock`/`crtc_htotal`/`crtc_vtotal` that still produces a near-zero frame duration. The `drm_mode_vrefresh()` check alone is not sufficient — the manual frame_ns check (once fixed to use `crtc_` fields) is the one that actually matters.

**Issue 3: Check ordering — zero fields before vrefresh (minor)**

```c
vrefresh = drm_mode_vrefresh(mode);
if (vrefresh > 1000)
    return -EINVAL;
if (!mode->crtc_clock || !mode->htotal || !mode->vtotal)
    return -EINVAL;
```

The zero-field check should come before calling `drm_mode_vrefresh()`, not after. When `htotal` or `vtotal` is zero, `drm_mode_vrefresh()` returns 0 (safe), so this doesn't cause a bug, but it's cleaner to validate basic field sanity before computing derived values. It also means the `drm_mode_vrefresh()` call is unnecessary overhead for degenerate modes.

**Issue 4: Interlace modes not accounted for (bug)**

`drm_calc_timestamping_constants()` halves `framedur_ns` for interlaced modes:

```c
if (mode->flags & DRM_MODE_FLAG_INTERLACE)
    framedur_ns /= 2;
```

The patch's manual calculation does not account for this:

```c
frame_ns = div_u64((u64)mode->htotal * mode->vtotal * 1000000ULL, mode->crtc_clock);
if (frame_ns < 1000000)
    return -EINVAL;
```

An interlaced mode with a frame duration of 1.5 ms would pass this check, but after halving in `drm_calc_timestamping_constants()`, the actual timer period would be 0.75 ms — still fast enough to cause problems.

**Issue 5: Comment is verbose (style, minor)**

The 8-line block comment is longer than typical kernel style for a simple validation. A 2-3 line comment would suffice, e.g.:

```c
/* Reject modes with frame duration < 1 ms to prevent hrtimer storms. */
```

**Suggestion:** Consider computing `framedur_ns` exactly as `drm_calc_timestamping_constants()` does (using `crtc_` fields, accounting for interlace), then rejecting if it's below the threshold. Alternatively, consider whether the validation belongs in `drm_calc_timestamping_constants()` itself or in the DRM core's mode validation, which would protect all drivers using vblank timers, not just vkms.

---
Generated by Claude Code Patch Reviewer