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

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

[Yao Yiqi]

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 console font is changed through KDFONTOP/KD_FONT_OP_SET, the new
font data and geometry are installed and vc_resize() is called. The
resulting redraw reaches the rotated bitops before the buffer is rebuilt:
hide_cursor() drives both con_cursor() -> soft_cursor() and fbcon_putc()
-> *_putcs(), and they index par->rotated.buf with the new cell size and
character codes. A stale, too-small buffer is read out of bounds.

KASAN reports, for example:

  BUG: KASAN: slab-out-of-bounds in soft_cursor
  Read of size 64 at addr ...
   memcpy
   soft_cursor
   cw_cursor
   fbcon_cursor
   hide_cursor
   redraw_screen
   vc_do_resize
   fbcon_do_set_font
   fbcon_set_font
   con_font_op
   vt_ioctl

Rebuild the buffer for the visible console right after the new font
geometry has been installed and before any redraw, as fbcon_switch() and
fbcon_modechanged() already do.

Unlike those callers, this one does not fall back to unrotated rendering
on failure. The "par->rotate = FB_ROTATE_UR; set_blitting_type(vc, info);"
fallback they use does not actually switch to unrotated rendering:
set_blitting_type() calls fbcon_set_rotation(), which immediately
re-derives par->rotate from par->p->con_rotate. Replicating that pattern
here would not change the outcome; a failed rebuild leaves
par->rotated.buf NULL, so the rotated ops skip drawing, and the following
patch retries the allocation on the next call.

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.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 01715873ea49..5cef18043bdb 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2432,6 +2432,17 @@ 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);
 
+	/*
+	 * The rotated font buffer is sized from the new font's width, height
+	 * and charcount. Rebuild it for the visible console before the redraw
+	 * below, as fbcon_switch() and fbcon_modechanged() already do. The
+	 * redraw reaches the rotated bitops through hide_cursor(), and those
+	 * index the buffer with the new cell size and character codes, so a
+	 * stale, too-small buffer is read out of bounds.
+	 */
+	if (con_is_visible(vc) && par->bitops->rotate_font)
+		par->bitops->rotate_font(info, vc);
+
 	if (resize) {
 		int cols, rows;
 
-- 
2.34.1

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

[Yao Yiqi]

fbcon_rotate_font() records the font and rotation it is about to handle in
par->rotated.fontdata and par->rotated.buf_rotate before calling
font_data_rotate(). If that allocation fails it clears par->rotated.buf,
but the cached font/rotation state 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 until
the font or the rotation is changed, even once memory is available again.

Also test par->rotated.buf in the early-out so a failed allocation is
retried the next time fbcon_rotate_font() runs.

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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 6cdbc96eeca6..6b148e0e81e6 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -21,7 +21,9 @@ 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 &&
+	    par->rotated.buf)
 		return 0;
 
 	par->rotated.fontdata = par->p->fontdata;
-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] fbcon: OOB read via dimension-swapped font loading
- [Medium] fbcon: broken fallback to unrotated rendering on rotate_font failure
--

commit 1bccc1e9cd74782edba7f76b0ad2e196a29fd948
Author: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon: Retry rotated font allocation after failure

fbcon_rotate_font() records the font and rotation it is about to handle in
par->rotated.fontdata and par->rotated.buf_rotate before calling
font_data_rotate(). If that allocation fails it clears par->rotated.buf,
but the cached font/rotation state already looks up to date, so the next
call takes the early-out and never retries.

Fixes: e4ef723d8975 ("fbcon: Avoid OOB font access if console rotation fails")
Cc: stable@vger.kernel.org

> The rotated putcs/cursor ops keep returning early on the NULL buffer and
> the console stays blank until the font or the rotation is changed, even
> once memory is available again.

[Severity: Medium]
Is the fallback to unrotated rendering on rotate_font() failure broken?

When par->bitops->rotate_font() fails, existing callers attempt to fall back
to unrotated rendering:

drivers/video/fbdev/core/fbcon.c:fbcon_init() {
    ...
	if (par->bitops->rotate_font && par->bitops->rotate_font(info, vc)) {
		par->rotate = FB_ROTATE_UR;
		set_blitting_type(vc, info);
	}
    ...
}

But set_blitting_type() invokes fbcon_set_rotation() unconditionally, which
overwrites the fallback state back to the originally requested rotation:

drivers/video/fbdev/core/fbcon.c:fbcon_set_rotation() {
    ...
	if (!(info->flags & FBINFO_MISC_TILEBLITTING) &&
	    par->p->con_rotate < 4)
		par->rotate = par->p->con_rotate;
    ...
}

Because the bitops are left as rotated, and par->rotated.buf is NULL, all
drawing operations abort and the screen remains completely blank:

drivers/video/fbdev/core/fbcon_ccw.c:ccw_putcs() {
    ...
	if (!par->rotated.buf)
		return;
    ...
}

Also, it appears the call to rotate_font() in fbcon_do_set_font() ignores
its return value entirely:

drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font() {
    ...
	if (con_is_visible(vc) && par->bitops->rotate_font)
		par->bitops->rotate_font(info, vc);
    ...
}

> diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
> index 6cdbc96eeca6f..6b148e0e81e66 100644
> --- a/drivers/video/fbdev/core/fbcon_rotate.c
> +++ b/drivers/video/fbdev/core/fbcon_rotate.c
> @@ -21,7 +21,9 @@ 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 &&
> +	    par->rotated.buf)
>  		return 0;

[Severity: High]
Does this early return still rely only on the fontdata pointer?

If a new font shares the same raw byte array (reused via font_data_is_equal)
but has different dimensions, it looks like it could bypass rotated buffer
reallocation because the fontdata pointer remains the same.

Rendering operations could then use the new, larger cellsize to index the
un-updated rotated buffer, resulting in an out-of-bounds read:

drivers/video/fbdev/core/fbcon_cw.c:cw_putcs_aligned() {
    ...
	src = par->rotated.buf + (scr_readw(s++) & charmask) * cellsize;
    ...
}

>  
>  	par->rotated.fontdata = par->p->fontdata;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014085102.497789-1-yaoyiqi3@huawei.com?part=2

Re: [PATCH 1/2] 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: dangling pointer in par->rotated.fontdata causes ABA vulnerability
- [High] fbcon: OOB read and UAF in fbcon_do_set_font error path
--

commit 40ac874a29ca63690bb8c7dfaad9a195bb52891d
Author: Yao Yiqi <yaoyiqi3@huawei.com>

fbcon: Rebuild rotated font buffer on font change

fbcon_rotate_font() caches a rotated copy of the console font in
par->rotated.buf. This commit fixes a slab-out-of-bounds read in
fbcon when changing the console font by explicitly rebuilding the
rotated font buffer before the redraw is triggered.

> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 615fa596b7cce..caad4e06944ea 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2431,6 +2431,17 @@ 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);
>  
> +	/*
> +	 * The rotated font buffer is sized from the new font's width, height
> +	 * and charcount. Rebuild it for the visible console before the redraw
> +	 * below, as fbcon_switch() and fbcon_modechanged() already do. The
> +	 * redraw reaches the rotated bitops through hide_cursor(), and those
> +	 * index the buffer with the new cell size and character codes, so a
> +	 * stale, too-small buffer is read out of bounds.
> +	 */
> +	if (con_is_visible(vc) && par->bitops->rotate_font)
> +		par->bitops->rotate_font(info, vc);
> +

[Severity: High]
If vc_resize() fails below this point (e.g., due to -ENOMEM from screen buffer
allocation), the err_out path reverts the console font metrics but leaves the
rotated buffer and pointer out of sync.

Could this leave par->rotated.buf sized for the new font, while the font
metrics are restored to the old font? If a subsequent call uses a smaller font,
the buffer could be allocated exactly to the smaller size. If vc_resize() then
fails, the metrics are restored to the larger font, which could cause a
slab-out-of-bounds read during subsequent redraws (such as cursor blink).

Furthermore, the err_out path drops the new font's refcount via
font_data_put(data), and the memory is subsequently freed by the caller.

Does this leave par->rotated.fontdata as a dangling pointer to this freed
memory? 

If a future font allocation reuses this exact slab address, the ABA logic in
fbcon_rotate_font():

	if (par->p->fontdata == par->rotated.fontdata &&
	    par->rotate == par->rotated.buf_rotate)
		return 0;

would incorrectly assume the font hasn't changed and skip rebuilding the
buffer, perpetuating the metric mismatch and leading to further out-of-bounds
accesses.

>  	if (resize) {
>  		int cols, rows;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014085102.497789-1-yaoyiqi3@huawei.com?part=1