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: fbcon: fix memory leak in error path of fbcon_do_set_font() Date: Tue, 26 May 2026 07:32:11 +1000 Message-ID: In-Reply-To: <20260525082741.600003-1-w15303746062@163.com> References: <20260525082741.600003-1-w15303746062@163.com> <20260525082741.600003-1-w15303746062@163.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Bug Analysis: Correct** The commit message clearly explains the root cause =E2=80=94 the `p->userfo= nt =3D old_userfont` assignment is inside the `if (userfont)` block in `err= _out`, so when a builtin font set (`userfont =3D=3D 0`) fails, the old user= font state is never restored. The kmemleak backtrace and the Fixes tag to `= a5a923038d70` are appropriate. **Code Change: Correct (for old code)** ```diff + /* Unconditionally restore the previous userfont state */ + p->userfont =3D old_userfont; + if (userfont) { - p->userfont =3D old_userfont; if (--REFCOUNT(data) =3D=3D 0) kfree(data - FONT_EXTRA_WORDS * sizeof(int)); } ``` Moving `p->userfont =3D old_userfont` outside the `if (userfont)` block is = the right fix. The restore should be unconditional regardless of the type o= f font that was being set, because the error path's job is to fully roll ba= ck state. The `if (userfont)` block that remains correctly handles freeing = the *new* font data (which only needs freeing if it was a user-allocated fo= nt), so that conditional stays correct. **Issues:** 1. **Not applicable to mainline.** The `[PATCH 7.0]` tag in the subject and= the note in the commit message acknowledge this, but the patch was sent to= dri-devel rather than stable@vger.kernel.org. It should target the 7.0.y s= table tree directly. Mainline's `fbcon_do_set_font()` now uses `font_data_t= ` with reference counting and has no `userfont` field at all. 2. **Comment is unnecessary.** The added `/* Unconditionally restore the pr= evious userfont state */` comment just restates what the code does. In the = kernel style, this would typically be omitted =E2=80=94 the variable name `= old_userfont` already makes the intent clear. Minor nit. 3. **Subject tag `[PATCH 7.0]` is non-standard.** For stable submissions, t= he standard practice is to send the patch with `Cc: stable@vger.kernel.org`= and optionally note the affected versions in the commit message body, not = use a version tag in the subject bracket. **Summary:** The fix itself is sound and addresses a real bug. It just need= s to go through the correct stable-tree submission process rather than bein= g sent as a mainline patch to dri-devel. --- Generated by Claude Code Patch Reviewer