[PATCH 7.0] fbdev: fbcon: fix memory leak in error path of fbcon_do_set_font()

[PATCH 7.0] fbdev: fbcon: fix memory leak in error path of fbcon_do_set_font()

[w15303746062]

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

[ Note: This issue was discovered on the 7.0 kernel. While the current
  mainline has already been refactored to use `font_data_t` (which 
  inadvertently resolved this bug), this vulnerability still actively 
  affects the 7.0 branch and older stable trees that rely on the legacy 
  userfont logic. This patch provides a targeted fix for these stable 
  branches. ]

When fbcon_do_set_font() fails (e.g., due to a vc_resize() failure under
fault injection), it jumps to the `err_out` label to roll back the
console state.

However, the restoration of the previous font state (`p->userfont =
old_userfont`) is erroneously placed inside the `if (userfont)` block.
If the failed operation was attempting to set the default builtin font
(`userfont == 0`), the restoration is completely skipped.

This causes a state machine corruption where `p->userfont` remains `0`
while `p->fontdata` still points to the previously allocated user font
memory. Later, when the console is destroyed (e.g., via VT_DISALLOCATE),
fbcon_free_font() fails to free this memory because its `if (p->userfont)`
check fails, resulting in a memory leak caught by kmemleak:

  unreferenced object 0xffff888127ea0000 (size 33296):
    comm "syz.0.8726", pid 33224, jiffies 4297754643
    hex dump (first 32 bytes):
      a6 e4 f9 dd 00 00 00 00 00 82 00 00 01 00 00 00  ................
      d2 09 6c bf 52 8a 7d d4 ef 1d 59 16 51 86 32 bf  ..l.R.}...Y.Q.2.
    backtrace (crc 4a0a57dd):
      ___kmalloc_large_node+0xe7/0x180 mm/slub.c:5214
      __kmalloc_large_node_noprof+0x29/0x130 mm/slub.c:5232
      __do_kmalloc_node mm/slub.c:5248 [inline]
      __kmalloc_noprof+0x5fc/0x7c0 mm/slub.c:5272
      kmalloc_noprof include/linux/slab.h:954 [inline]
      fbcon_set_font+0x431/0xa60 drivers/video/fbdev/core/fbcon.c:2525
      con_font_set drivers/tty/vt/vt.c:4918 [inline]
      con_font_op+0x94d/0xe80 drivers/tty/vt/vt.c:4958
      vt_k_ioctl drivers/tty/vt/vt_ioctl.c:472 [inline]
      vt_ioctl+0x63c/0x2ee0 drivers/tty/vt/vt_ioctl.c:743

Fix this by moving the `p->userfont = old_userfont` assignment outside
the `if (userfont)` block so that the terminal state is unconditionally
and correctly restored regardless of which font setting triggered the
error.

Fixes: a5a923038d70 ("fbdev: fbcon: Properly revert changes when vc_resize() failed")
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
 drivers/video/fbdev/core/fbcon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 666261ae59d8..a38545dc8416 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2461,8 +2461,10 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	p->fontdata = old_data;
 	vc->vc_font.data = old_data;
 
+	/* Unconditionally restore the previous userfont state */
+	p->userfont = old_userfont;
+
 	if (userfont) {
-		p->userfont = old_userfont;
 		if (--REFCOUNT(data) == 0)
 			kfree(data - FONT_EXTRA_WORDS * sizeof(int));
 	}
-- 
2.34.1

Claude review: fbdev: fbcon: fix memory leak in error path of fbcon_do_set_font()

[Claude Code Review Bot]

Overall Series Review

Subject: fbdev: fbcon: fix memory leak in error path of fbcon_do_set_font()
Author: w15303746062@163.com
Patches: 1
Reviewed: 2026-05-26T07:32:10.831353

---

This is a single patch (not a series) that fixes a real memory leak bug in `fbcon_do_set_font()`. The bug analysis is correct: in the error path (`err_out`), the `p->userfont = old_userfont` restoration was inside the `if (userfont)` block, meaning when setting a builtin font failed (i.e., `userfont == 0`), the userfont flag was never restored. This could leave `p->userfont == 0` while `p->fontdata` still pointed to a previously-allocated user font, causing `fbcon_free_font()` to skip freeing it on console teardown.

**However, this patch does not apply to current mainline (drm-next).** As the author acknowledges, mainline has been refactored to use `font_data_t` with proper reference counting (`font_data_get()`/`font_data_put()`), which completely eliminated the `userfont` flag and the associated bug class. I confirmed that the current tree has zero references to `userfont` in `fbcon.c`, and the error path now simply calls `font_data_put(data)` to release the new font data.

**This patch is only applicable to stable/older kernels (7.0 and earlier)** that still use the legacy `userfont` logic. It should be submitted to the stable mailing list (stable@vger.kernel.org) with the appropriate stable tree version targeting, not to dri-devel as a mainline patch.

**Verdict: The fix is logically correct for the old code, but it is NAK for mainline since the code it modifies no longer exists. It needs to be resubmitted targeting the appropriate stable branches.**

---
Generated by Claude Code Patch Reviewer

Claude review: fbdev: fbcon: fix memory leak in error path of fbcon_do_set_font()

[Claude Code Review Bot]

Patch Review

**Bug Analysis: Correct**

The commit message clearly explains the root cause — the `p->userfont = old_userfont` assignment is inside the `if (userfont)` block in `err_out`, so when a builtin font set (`userfont == 0`) fails, the old userfont 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 = old_userfont;
+
 	if (userfont) {
-		p->userfont = old_userfont;
 		if (--REFCOUNT(data) == 0)
 			kfree(data - FONT_EXTRA_WORDS * sizeof(int));
 	}
```

Moving `p->userfont = old_userfont` outside the `if (userfont)` block is the right fix. The restore should be unconditional regardless of the type of font that was being set, because the error path's job is to fully roll back state. The `if (userfont)` block that remains correctly handles freeing the *new* font data (which only needs freeing if it was a user-allocated font), 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 stable 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 previous userfont state */` comment just restates what the code does. In the kernel style, this would typically be omitted — the variable name `old_userfont` already makes the intent clear. Minor nit.

3. **Subject tag `[PATCH 7.0]` is non-standard.** For stable submissions, the 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 needs to go through the correct stable-tree submission process rather than being sent as a mainline patch to dri-devel.

---
Generated by Claude Code Patch Reviewer