From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: fbdev: udlfb: avoid divide-by-zero on FBIOPUT_VSCREENINFO Date: Sun, 12 Apr 2026 11:08:13 +1000 Message-ID: In-Reply-To: <2026040945-glare-devouring-f242@gregkh> References: <2026040945-glare-devouring-f242@gregkh> <2026040945-glare-devouring-f242@gregkh> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Issue being fixed:** In `dlfb_set_video_mode()` at line 271 of `udlfb.c`,= the driver computes: ```c 200*1000*1000/var->pixclock ``` If `var->pixclock` is zero, this is a divide-by-zero crash. The `var` struc= t originates from userspace via `FBIOPUT_VSCREENINFO`, so a malicious or bu= ggy application can trigger this. The fbdev core calls `check_var` before `= set_par`, so validating in `dlfb_ops_check_var()` is the correct place to c= atch it early. **The fix:** ```c + if (!var->pixclock) + return -EINVAL; ``` This is placed at the top of `dlfb_ops_check_var()`, before `dlfb_var_color= _format(var)` and `fb_var_to_videomode()`. This is correct =E2=80=94 there'= s no reason to proceed with any mode validation if pixclock is zero, since = the mode would be unusable regardless. **Minor observations:** 1. The placement before `dlfb_var_color_format()` is fine =E2=80=94 that fu= nction only touches color fields, not pixclock =E2=80=94 but placing it fir= st is slightly more defensive and avoids unnecessary work on an invalid inp= ut. Good choice. 2. There's a second call site for `dlfb_set_video_mode()` at line 1106 (in = `dlfb_ops_open()` which re-sets the current mode). That path uses `info->va= r` which should have already passed through `check_var`, so it's covered. C= orrect. 3. The `Fixes:` tag pointing to commit 59277b679f8b ("Staging: udlfb: add d= ynamic modeset support") is appropriate =E2=80=94 that's the commit that in= troduced `dlfb_ops_check_var()` and the division via `dlfb_set_video_mode()= `. 4. The `Assisted-by: gregkh_clanker_t1000` tag is a non-standard tag (not i= n the usual kernel tag vocabulary). This likely indicates AI assistance. Th= ere's been ongoing discussion in the kernel community about the appropriate= tag for AI-assisted contributions; this may want to follow whatever conven= tion the community settles on, but it's not a blocking issue. **Verdict: Reviewed-by worthy.** The patch is correct, minimal, and well-ta= rgeted. No issues found. --- Generated by Claude Code Patch Reviewer