[PATCH v4 0/3] fbcon: fix rotated font buffer handling on font change

[PATCH v4 0/3] fbcon: fix rotated font buffer handling on font change

[Yao Yiqi]

Fix out-of-bounds reads of the rotated font buffer when the console
font changes on a rotated console (fbcon=rotate:N). KDFONTOP installs
the new geometry and vc_resize() redraws before par->rotated.buf is
updated, so the old buffer is indexed with the new cell size.

Patch 1 keys the rotated font cache on the font geometry. Patch 2
rebuilds it on font changes, restoring the old buffer if the change
fails. Patch 3 retries a failed allocation.

Changes since v3: roll back via the saved buffer instead of re-running
rotate_font(), abort the font change on rebuild failure, and guard the
rotated state with CONFIG_FRAMEBUFFER_CONSOLE_ROTATION.

v1: https://lore.kernel.org/all/20261014085102.497789-1-yaoyiqi3@huawei.com/

Yao Yiqi (3):
  fbcon: Include font geometry in the rotated font cache
  fbcon: Rebuild rotated font buffer on font change
  fbcon: Retry rotated font allocation after failure

 drivers/video/fbdev/core/fbcon.c        | 50 +++++++++++++++++++++++++++++++++
 drivers/video/fbdev/core/fbcon.h        |  9 ++++---
 drivers/video/fbdev/core/fbcon_rotate.c | 10 +++++++-
 3 files changed, 65 insertions(+), 4 deletions(-)

-- 
2.56.0

[PATCH v4 1/3] fbcon: Include font geometry in the rotated font cache

[Yao Yiqi]

From: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon_rotate_font() caches a rotated copy of the console font and
skips the rebuild when the fontdata pointer and the rotation match.
Raw font data carries no geometry, though: width, height and
charcount live in vc->vc_font. fbcon_set_font() deduplicates uploads
by byte equality, so the same bytes re-uploaded with different
dimensions keep the fontdata pointer while the geometry changes, and
the early-out skips the rebuild.

The rotated cell size depends on the geometry (rotation swaps width
and height), so the stale buffer is indexed with the new cell size.
Re-uploading a 4x16 font as 16x8 on a CW-rotated console, for
example, doubles the rotated cell size and reads past the end of the
old buffer.

Record the geometry the buffer was built from and compare it in the
early-out. The pointer-only key used to be backed by an explicit
width check in the deduplication, which commit 1e3c49aa03fb
("lib/fonts: Compare font data for equality with font_data_is_equal()")
dropped.

Fixes: 6cc50e1c5b57 ("[PATCH] fbcon: Console Rotation - Add support to rotate font bitmap")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Yiqi <yaoyiqi3@huawei.com>
---
 drivers/video/fbdev/core/fbcon.h        | 9 ++++++---
 drivers/video/fbdev/core/fbcon_rotate.c | 9 ++++++++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 407d207b14f1..bfbd70f8b1cd 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -89,10 +89,13 @@ struct fbcon_par {
 	char  *cursor_data;
 #ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
 	struct {
-		font_data_t *fontdata;  /* source font */
-		u8 *buf;                /* rotated glyphs */
+		font_data_t *fontdata;       /* source font */
+		u8 *buf;                     /* rotated glyphs */
 		size_t bufsize;
-		int buf_rotate;         /* rotation of buf */
+		int buf_rotate;              /* rotation of buf */
+		unsigned int buf_width;      /* geometry of source font */
+		unsigned int buf_height;
+		unsigned int buf_charcount;
 	} rotated;
 #endif
 	u8    *cursor_src;
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 6cdbc96eeca6..660a3fa81bae 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -21,11 +21,18 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
 	unsigned char *buf;
 	int ret;
 
-	if (par->p->fontdata == par->rotated.fontdata && par->rotate == par->rotated.buf_rotate)
+	if (par->p->fontdata == par->rotated.fontdata &&
+	    par->rotate == par->rotated.buf_rotate &&
+	    vc->vc_font.width == par->rotated.buf_width &&
+	    vc->vc_font.height == par->rotated.buf_height &&
+	    vc->vc_font.charcount == par->rotated.buf_charcount)
 		return 0;
 
 	par->rotated.fontdata = par->p->fontdata;
 	par->rotated.buf_rotate = par->rotate;
+	par->rotated.buf_width = vc->vc_font.width;
+	par->rotated.buf_height = vc->vc_font.height;
+	par->rotated.buf_charcount = vc->vc_font.charcount;
 
 	if (info->fbops->fb_sync)
 		info->fbops->fb_sync(info);
-- 
2.56.0

[PATCH v4 2/3] fbcon: Rebuild rotated font buffer on font change

[Yao Yiqi]

From: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon_rotate_font() caches a rotated copy of the console font in
par->rotated.buf. The buffer is sized from the font's width, height
and charcount, and is rebuilt in fbcon_init(), fbcon_switch() and
fbcon_modechanged(), but not in fbcon_do_set_font().

When the font is changed through KDFONTOP, the new geometry is
installed and vc_resize() redraws before the buffer is rebuilt. The
rotated putcs/cursor ops then index the old buffer with the new cell
size; KASAN reports a slab-out-of-bounds read in soft_cursor().

Rebuild the buffer for the visible console before the redraw, as the
other callers do: select the console's blitting state with
set_blitting_type() (which also uploads the new font's map on
tile-blitting hardware), then rebuild. Background consoles keep
their rebuild at fbcon_switch() time, as the buffer is shared per
framebuffer and must not be updated for a console that is not being
displayed.

Keep the old buffer out of the rebuild so that it can be restored if
vc_resize() fails: clear par->rotated.buf before calling
rotate_font(), which then allocates a fresh buffer. On failure,
err_out puts the old buffer back and re-selects the blitting state
so tile-blitting hardware gets the restored font's map; the font
change fails as a whole, and the error path never runs rotate_font()
again. The retry added by the next patch still covers
fbcon_switch() and fbcon_modechanged(), which cannot roll back.

Unlike the other callers, this one does not fall back to unrotated
rendering on failure: their "par->rotate = FB_ROTATE_UR;
set_blitting_type(vc, info);" pattern is ineffective, because
set_blitting_type() re-derives par->rotate from par->p->con_rotate.
Repairing that fallback is left to separate work.

Fixes: 6cc50e1c5b57 ("[PATCH] fbcon: Console Rotation - Add support to rotate font bitmap")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Yiqi <yaoyiqi3@huawei.com>
---
Changes in v4:

- Restore the saved rotated buffer on failure instead of re-running
  rotate_font(), so that a second allocation failure cannot leave the
  console in a third state.
- Keep the old buffer out of the rebuild by allocating a fresh one.
- Abort the font change when the rebuild fails.
- Guard the rotated state with CONFIG_FRAMEBUFFER_CONSOLE_ROTATION.
---
 drivers/video/fbdev/core/fbcon.c | 50 ++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 01715873ea49..7605e4237ca5 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2414,6 +2414,11 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	font_data_t *old_fontdata = p->fontdata;
 	const u8 *old_data = vc->vc_font.data;
 	unsigned short old_hi_font_mask = vc->vc_hi_font_mask;
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+	u8 *saved_rotated_buf = NULL;
+	size_t saved_rotated_bufsize = 0;
+	bool rotated_saved = false;
+#endif
 
 	font_data_get(data);
 
@@ -2432,6 +2437,29 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	else if (!vc->vc_hi_font_mask && charcount == 512)
 		set_vc_hi_font(vc, true);
 
+	/*
+	 * Select the console's blitting state and update par->rotated to
+	 * match the new font, so that the rendering below does not use a
+	 * cache that was built for the old font. Clear the buffer so the
+	 * rebuild allocates a fresh one; the old buffer is kept until the
+	 * font change can no longer fail.
+	 */
+	if (con_is_visible(vc)) {
+		set_blitting_type(vc, info);
+		if (par->bitops->rotate_font) {
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+			saved_rotated_buf = par->rotated.buf;
+			saved_rotated_bufsize = par->rotated.bufsize;
+			par->rotated.buf = NULL;
+			par->rotated.bufsize = 0;
+			rotated_saved = true;
+#endif
+			ret = par->bitops->rotate_font(info, vc);
+			if (ret)
+				goto err_out;
+		}
+	}
+
 	if (resize) {
 		int cols, rows;
 
@@ -2448,12 +2476,27 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 		update_screen(vc);
 	}
 
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+	if (rotated_saved)
+		kfree(saved_rotated_buf);
+#endif
 	if (old_fontdata)
 		font_data_put(old_fontdata);
 
 	return 0;
 
 err_out:
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+	/*
+	 * Drop the buffer built for the new font and restore the saved
+	 * one.
+	 */
+	if (rotated_saved) {
+		kfree(par->rotated.buf);
+		par->rotated.buf = saved_rotated_buf;
+		par->rotated.bufsize = saved_rotated_bufsize;
+	}
+#endif
 	p->fontdata = old_fontdata;
 	vc->vc_font.data = old_data;
 	vc->vc_font.width = old_width;
@@ -2466,6 +2509,13 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	else if (!old_hi_font_mask && vc->vc_hi_font_mask)
 		set_vc_hi_font(vc, false);
 
+	/*
+	 * Re-select the blitting type, which re-uploads the font map on
+	 * tile-blitting hardware.
+	 */
+	if (con_is_visible(vc))
+		set_blitting_type(vc, info);
+
 	font_data_put(data);
 
 	return ret;
-- 
2.56.0

[PATCH v4 3/3] fbcon: Retry rotated font allocation after failure

[Yao Yiqi]

From: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon_rotate_font() records the font, its geometry and the rotation in
par->rotated before calling font_data_rotate(). If the allocation
fails, the error path clears par->rotated.buf, but the recorded key
already looks up to date, so the next call takes the early-out and
never retries. The rotated putcs/cursor ops keep returning early on
the NULL buffer, and the console stays blank even after memory is
available again.

Also test par->rotated.buf in the early-out, so a failed allocation
is retried on the next call: at the next fbcon_switch(),
fbcon_modechanged() or font change.

Fixes: e4ef723d8975 ("fbcon: Avoid OOB font access if console rotation fails")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Yiqi <yaoyiqi3@huawei.com>
---
 drivers/video/fbdev/core/fbcon_rotate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 660a3fa81bae..7afd63dcee7b 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -25,7 +25,8 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
 	    par->rotate == par->rotated.buf_rotate &&
 	    vc->vc_font.width == par->rotated.buf_width &&
 	    vc->vc_font.height == par->rotated.buf_height &&
-	    vc->vc_font.charcount == par->rotated.buf_charcount)
+	    vc->vc_font.charcount == par->rotated.buf_charcount &&
+	    par->rotated.buf)
 		return 0;
 
 	par->rotated.fontdata = par->p->fontdata;
-- 
2.56.0

Re: [PATCH v4 2/3] fbcon: Rebuild rotated font buffer on font change

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] fbcon: incomplete par->rotated rollback on error in fbcon_do_set_font()
- [High] fbcon: incomplete par->rotated state save and restore in fbcon_do_set_font()
--

commit 40279fe676962065434b918b61507d77c5b4a060
Author: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon: Rebuild rotated font buffer on font change

This patch rebuilds the rotated font buffer in fbcon_do_set_font() when
changing fonts on the visible console so that vc_resize() does not render
using a rotated font cache sized for the old font. If rotation or resizing
fails, it restores the saved rotated buffer and rolls back the font change.

> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 01715873ea49b..7605e4237ca5b 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2414,6 +2414,11 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
>  	font_data_t *old_fontdata = p->fontdata;
>  	const u8 *old_data = vc->vc_font.data;
>  	unsigned short old_hi_font_mask = vc->vc_hi_font_mask;
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
> +	u8 *saved_rotated_buf = NULL;
> +	size_t saved_rotated_bufsize = 0;
> +	bool rotated_saved = false;
> +#endif
>  
>  	font_data_get(data);
>  
> @@ -2432,6 +2437,29 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
>  	else if (!vc->vc_hi_font_mask && charcount == 512)
>  		set_vc_hi_font(vc, true);
>  
> +	/*
> +	 * Select the console's blitting state and update par->rotated to
> +	 * match the new font, so that the rendering below does not use a
> +	 * cache that was built for the old font. Clear the buffer so the
> +	 * rebuild allocates a fresh one; the old buffer is kept until the
> +	 * font change can no longer fail.
> +	 */
> +	if (con_is_visible(vc)) {
> +		set_blitting_type(vc, info);
> +		if (par->bitops->rotate_font) {
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
> +			saved_rotated_buf = par->rotated.buf;
> +			saved_rotated_bufsize = par->rotated.bufsize;
> +			par->rotated.buf = NULL;
> +			par->rotated.bufsize = 0;
> +			rotated_saved = true;
> +#endif
> +			ret = par->bitops->rotate_font(info, vc);

[Severity: High]
Can clearing only par->rotated.buf and par->rotated.bufsize here cause the
active rotated font buffer to be freed and lost when re-setting the same
font?

In drivers/video/fbdev/core/fbcon_rotate.c, fbcon_rotate_font() checks
whether the cache metadata matches before allocating a new buffer:

drivers/video/fbdev/core/fbcon_rotate.c:fbcon_rotate_font() {
	if (par->p->fontdata == par->rotated.fontdata &&
	    par->rotate == par->rotated.buf_rotate &&
	    vc->vc_font.width == par->rotated.buf_width &&
	    vc->vc_font.height == par->rotated.buf_height &&
	    vc->vc_font.charcount == par->rotated.buf_charcount)
		return 0;
}

At this commit, before the subsequent commit 4c61e799dc92 ("fbcon: Retry
rotated font allocation after failure") adds a check for par->rotated.buf,
re-applying a font with the same fontdata and geometry (for example via
fbcon_set_font() deduplication or fbcon_set_def_font()) hits this early
return with par->rotated.buf still NULL. fbcon_do_set_font() then calls
kfree(saved_rotated_buf), leaving par->rotated.buf NULL.

> +			if (ret)
> +				goto err_out;
> +		}
> +	}
> +
>  	if (resize) {
>  		int cols, rows;
>  
> @@ -2448,12 +2476,27 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
>  		update_screen(vc);
>  	}
>  
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
> +	if (rotated_saved)
> +		kfree(saved_rotated_buf);
> +#endif
>  	if (old_fontdata)
>  		font_data_put(old_fontdata);
>  
>  	return 0;
>  
>  err_out:
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
> +	/*
> +	 * Drop the buffer built for the new font and restore the saved
> +	 * one.
> +	 */
> +	if (rotated_saved) {
> +		kfree(par->rotated.buf);
> +		par->rotated.buf = saved_rotated_buf;
> +		par->rotated.bufsize = saved_rotated_bufsize;
> +	}
> +#endif

[Severity: High]
What happens to the cache key fields in par->rotated when rotate_font() or
vc_resize() fails and jumps to err_out?

In fbcon_rotate_font(), the cache key fields are updated with the new
font's pointer and geometry before font_data_rotate() is called:

drivers/video/fbdev/core/fbcon_rotate.c:fbcon_rotate_font() {
	...
	par->rotated.fontdata = par->p->fontdata;
	par->rotated.buf_rotate = par->rotate;
	par->rotated.buf_width = vc->vc_font.width;
	par->rotated.buf_height = vc->vc_font.height;
	par->rotated.buf_charcount = vc->vc_font.charcount;
	...
	buf = font_data_rotate(...);
}

If font_data_rotate() or the subsequent vc_resize() in fbcon_do_set_font()
fails, err_out restores par->rotated.buf and par->rotated.bufsize to the
old font's buffer and restores p->fontdata and vc->vc_font to the old font,
but leaves par->rotated.{fontdata,buf_rotate,buf_width,buf_height,
buf_charcount} set to the failed new font's metadata.

Because par->rotated.buf is restored to the non-NULL saved_rotated_buf,
even with commit 4c61e799dc92 ("fbcon: Retry rotated font allocation after
failure") applied, switching via fbcon_switch() to another console that
uses that same font and larger geometry (or a subsequent font operation
that reuses the freed font_data_t address with the larger geometry) will
see all cache key fields match and par->rotated.buf != NULL in
fbcon_rotate_font(). It will return early without rebuilding the buffer,
and the rotated blitting and cursor callbacks (cw_putcs/ccw_putcs/ud_putcs
and cw_cursor/ccw_cursor/ud_cursor) will index the smaller
saved_rotated_buf using the larger cell size, triggering a
slab-out-of-bounds read.

Would saving and clearing the entire par->rotated struct before calling
rotate_font(), and restoring the whole struct in err_out, avoid both
issues?

[ ... ]

>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019090206.1094958-1-yyaqia@gmail.com?part=2

Re: [PATCH v4 3/3] fbcon: Retry rotated font allocation after failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fbcon: stale rotated font cache key in fbcon_rotate_font() on error rollback
--

commit 4c61e799dc92607244ca176d7070e7e3d2fde5e4
Author: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon: Retry rotated font allocation after failure

This commit checks par->rotated.buf in the early-out condition of
fbcon_rotate_font() so that a failed rotated font allocation is retried on
the next call instead of leaving the console blank.

> diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
> index 660a3fa81baed..7afd63dcee7be 100644
> --- a/drivers/video/fbdev/core/fbcon_rotate.c
> +++ b/drivers/video/fbdev/core/fbcon_rotate.c
> @@ -25,7 +25,8 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
>  	    par->rotate == par->rotated.buf_rotate &&
>  	    vc->vc_font.width == par->rotated.buf_width &&
>  	    vc->vc_font.height == par->rotated.buf_height &&
> -	    vc->vc_font.charcount == par->rotated.buf_charcount)
> +	    vc->vc_font.charcount == par->rotated.buf_charcount &&
> +	    par->rotated.buf)
>  		return 0;
>  
>  	par->rotated.fontdata = par->p->fontdata;

[Severity: High]
Should fbcon_rotate_font() update the cache key fields in par->rotated only
after font_data_rotate() succeeds, rather than relying solely on checking
par->rotated.buf in the early-out?

Because fbcon_rotate_font() records par->rotated.fontdata, buf_rotate,
buf_width, buf_height, and buf_charcount before calling font_data_rotate(),
those fields still get overwritten with the new font's metadata when
font_data_rotate() fails.

In drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font(), the caller saves
par->rotated.buf before calling rotate_font() and restores the old buffer on
failure:

drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font() {
	if (con_is_visible(vc)) {
		set_blitting_type(vc, info);
		if (par->bitops->rotate_font) {
#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
			saved_rotated_buf = par->rotated.buf;
			saved_rotated_bufsize = par->rotated.bufsize;
			par->rotated.buf = NULL;
			par->rotated.bufsize = 0;
			rotated_saved = true;
#endif
			ret = par->bitops->rotate_font(info, vc);
			if (ret)
				goto err_out;
		}
	}
	...
err_out:
	if (rotated_saved) {
		kfree(par->rotated.buf);
		par->rotated.buf = saved_rotated_buf;
		par->rotated.bufsize = saved_rotated_bufsize;
	}
}

If rotate_font() (or the subsequent vc_resize()) fails in fbcon_do_set_font()
when switching a visible console from a smaller font (e.g. 8x8) to a larger
font (e.g. 16x32), err_out restores par->rotated.buf to the smaller font's
non-NULL saved_rotated_buf while leaving par->rotated.{fontdata, buf_rotate,
buf_width, buf_height, buf_charcount} set to the failed larger font's
metadata.

If another console already uses that same larger font (or if changing to a
built-in/shared font matching that stale key is retried), the next
fbcon_switch() -> fbcon_rotate_font() sees all key fields match and
par->rotated.buf != NULL, so it returns 0 without reallocating or rotating
the buffer. Could the rotated rendering callbacks (cw_putcs(), ccw_putcs(),
ud_putcs(), cw_cursor(), ccw_cursor(), ud_cursor()) then read out of bounds
from the smaller restored par->rotated.buf using the larger font's geometry?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019090206.1094958-1-yyaqia@gmail.com?part=3