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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fbcon: UAF in panic handler due to dangling par->rotated.buf
--

commit e32477b2f73cd204764bac2654d00076bb8bd3ba
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. 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().

> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 01715873ea49b..d90a2ad45053c 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);
>  
> +	/*
> +	 * 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.
> +	 */
> +	if (con_is_visible(vc)) {
> +		set_blitting_type(vc, info);
> +		if (par->bitops->rotate_font)
> +			par->bitops->rotate_font(info, vc);

[Severity: High]
Does this code expose a race condition with console_flush_on_panic() that could
lead to a use-after-free?

When fbcon_rotate_font() is called, it passes par->rotated.buf to
font_data_rotate():

drivers/video/fbdev/core/fbcon_rotate.c:fbcon_rotate_font() {
    ...
	buf = font_data_rotate(par->rotated.fontdata, vc->vc_font.width,
			       vc->vc_font.height, vc->vc_font.charcount,
			       par->rotated.buf_rotate, par->rotated.buf,
			       &par->rotated.bufsize);
    ...
}

If a reallocation is needed, font_data_rotate() frees the old buffer before
returning the new one:

lib/fonts/font_rotate.c:font_data_rotate() {
    ...
		kfree(buf);
		buf = dst;
    ...
}

During this time, the global par->rotated.buf pointer remains dangling.
If a kernel panic occurs in this window, console_flush_on_panic() could
bypass the console lock and invoke fbcon write operations.

Rendering operations like ccw_putcs() check the pointer:

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

Because par->rotated.buf hasn't been set to NULL or updated to the new
buffer yet, this check would evaluate to false, leading to a read from freed
memory which could abort the panic handler and lose important debug logs.

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