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

[sashiko-bot@kernel.org]

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