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

[Claude Code Review Bot <claude-review@example.com>]

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