Re: [PATCH V2] fbdev: Fix out-of-bounds access when rotating console after font resize

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The exact same OOB access remains when the screen resolution is shrunk while a text selection is active or when changing the font size via ioctl(KDFONTOP) on a rotated console.
- [High] The fallback logic for when rotate_font() fails is fundamentally broken, leading to a silent failure to render text.
--

--- Patch [1]: [PATCH V2] fbdev: Fix out-of-bounds access when rotating console after font resize ---
commit 8b1b772d608d9f550c7052854497b048b2d6b16a
Author: Zizhi Wo <wozizhi@huawei.com>

fbdev: Fix out-of-bounds access when rotating console after font resize

This commit addresses an out-of-bounds access when changing console
rotation and font size. It ensures clear_selection() is called earlier
and a correctly sized buffer is allocated in time by calling
rotate_font() before vc_resize().

> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 9f5c4c101581d..3361479d139d3 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -2641,9 +2641,16 @@ static void fbcon_modechanged(struct fb_info *info)
>  	    fbcon_info_from_console(par->currcon) != info)
>  		return;
>  
> +	clear_selection();
> +

[Severity: Critical]
This is a pre-existing issue, but does moving clear_selection() here still
leave us vulnerable if the screen resolution is shrunk while a text selection
is active, or if the font size is increased via ioctl(KDFONTOP)?

When clear_selection() calls highlight(), it iterates through the selected
text. If the font size is increased or the screen resolution is smaller,
wouldn't it still use the old, larger vc->vc_cols boundary since vc_resize()
hasn't updated vc_cols yet?

In drivers/video/fbdev/core/fbcon_ccw.c:ccw_putcs(), this could result in:

	image.dy = vyres - ((xx + count) * vc->vc_font.width);

If (xx + count) reaches the old max columns, could this cause an underflow
to a massive value, leading to another out-of-bounds memory access?

>  	p = &fb_display[vc->vc_num];
>  	set_blitting_type(vc, info);
>  
> +	if (par->bitops->rotate_font && par->bitops->rotate_font(info, vc)) {
> +		par->rotate = FB_ROTATE_UR;
> +		set_blitting_type(vc, info);
> +	}
> +

[Severity: High]
This is a pre-existing issue, but is this fallback logic effective if
rotate_font() fails?

The code attempts to fall back to an upright rotation (FB_ROTATE_UR), but
doesn't set_blitting_type() internally call fbcon_set_rotation(info)?

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;
    ...
}

It appears this would unconditionally reset par->rotate back to the original
con_rotate. Since par->rotated.buf would remain NULL, might the drawing
routines silently return without rendering any text?

>  	if (con_is_visible(vc)) {
>  		var_to_display(p, &info->var, info);
>  		cols = FBCON_SWAP(par->rotate, info->var.xres, info->var.yres);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722062216.2574546-1-wozizhi@huaweicloud.com?part=1