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

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

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

[BUG]
Recently, we encountered a KASAN warning as follows:

BUG: KASAN: slab-out-of-bounds in ccw_putcs+0x8bd/0xa80
Read of size 1 at addr ff11000110067100 by task bash/1209
CPU: 10 UID: 0 PID: 1209 Comm: bash Not tainted 7.2.0-rc3 #69 PREEMPT(full)
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-4.fc41 04/01/2014
 Call Trace:
  <TASK>
  ...
  kasan_report+0xf0/0x120
  ? ccw_putcs+0x8bd/0xa80
  ccw_putcs+0x8bd/0xa80
  ? __pfx_ccw_putcs+0x10/0x10
  fbcon_putcs+0x338/0x410
  ? __pfx_ccw_putcs+0x10/0x10
  do_update_region+0x21d/0x450
  invert_screen+0x29d/0x5e0
  ? __kmalloc_noprof+0x493/0x640
  ? vc_do_resize+0x17c/0xe50
  clear_selection+0x4c/0x60
  vc_do_resize+0xaee/0xe50
  fbcon_modechanged+0x2bd/0x640
  rotate_all_store+0x298/0x380
  ...

reproduce:
1) issue two ioctls: first a KDFONTOP ioctl with op.op = KD_FONT_OP_SET,
op.width = 1 and op.height = 1, then a TIOCL_SETSEL ioctl
2) echo 2 > /sys/devices/virtual/graphics/fbcon/rotate_all
3) issue two ioctls: first a KDFONTOP ioctl with op.op = KD_FONT_OP_SET,
op.width = 8 and op.height = 1, then a TIOCL_SETSEL ioctl
4) echo 3 > /sys/devices/virtual/graphics/fbcon/rotate_all

[CAUSE]
The root cause is that fbcon_modechanged() first sets the current rotate's
corresponding ops. Subsequently, during vc_resize(), it may trigger
clear_selection(), and in fbcon_putcs->ccw_putcs[rotate=3], this can result
in an out-of-bounds access to "src". This happens because par->rotated.buf
is reallocated in fbcon_rotate_font():
1) When rotate=2, its size is (width + 7) / 8 * height
2) When rotate=3, its size is (height + 7) / 8 * width

And the call to fbcon_rotate_font() occurs after clear_selection(). In
other words, the fontbuffer is allocated using the size calculated from the
previous rotation 2, but before reallocating it with the new size,
con_putcs is already using the new rotation 3:

rotate_all_store
 fbcon_rotate_all
  fbcon_set_all_vcs
   fbcon_modechanged
    set_blitting_type
    ...
     par->bitops = &ccw_fbcon_bitops
    vc_resize
    ...
     clear_selection
      highlight
      ...
       do_update_region
	fbcon_putcs
	...
	 image.dy = vyres - ((xx + count) * vc->vc_font.width) [1]  // overflow!
	 ccw_putcs_aligned
	  // old buf size is still being used during the read!
	  src = par->rotated.buf + (scr_readw(s--) & charmask) * cellsize
	  fb_pad_aligned_buffer----[src KASAN!!!]	[2]
	  info->fbops->fb_imageblit(info, image)
	   sys_imageblit
	    fb_imageblit
	     fb_address_forward
	      // offset: image->dy * bits_per_line + image->dx * bpp
	      unsigned int bits = (unsigned int)adr->bits + offset
	      adr->address += (bits & ~(BITS_PER_LONG - 1u)) / BITS_PER_BYTE	[3]
	     fb_bitmap_imageblit
	     ...
	      fb_read_offset	// page fault!	[4]
    update_screen
     redraw_screen
     ...
      ccw_cursor
       soft_cursor
        memcpy(src, image->data, dsize)----[src KASAN again!!!]	[5]
     fbcon_switch
      fbcon_rotate_font
       font_data_rotate
	dst = kmalloc_array(charcount, d_cellsize, GFP_KERNEL)
       // the new size is allocated only here!
       par->rotated.buf = buf	[6]

[FIX]
A fairly obvious approach is to follow fbcon_switch(): in
fbcon_modechanged(), call rotate_font() before vc_resize() so that a
correctly sized buffer is allocated in time, as done in [6]. This fix is
necessary, but it is not sufficient on its own.

In [1] it causes an image.dy overflow (ccw_putcs: vyres = 768,
image.dy = 4294967040), because vc_cols has not been updated in time at
this point (it is likewise only updated after clear_selection()). This
allows (xx + count) * width to exceed vyres, causing image.dy to overflow.
Subsequently, address in [3] is incremented by an even larger amount, which
triggers a page fault at [4].

Therefore, a second fix is required in combination with the first: move
clear_selection() earlier, before set_blitting_type() in
fbcon_set_all_vcs(), to prevent the out-of-bounds access. fbcon_rotate()
has a similar problem, so add the same clear there. Since vc_is_sel() is
not exported, the fbdev side is currently forced to call clear_selection()
unconditionally, causing the global selection to be cleared prematurely.
And this will not cause any other significant impact.

Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
v2:
Fixed the issue by calling clear_selection() earlier, and updated the
related description in the commit message.

v1: https://lore.kernel.org/all/20250905024340.337521-1-wozizhi@huaweicloud.com/
---
 drivers/video/fbdev/core/fbcon.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 9f5c4c101581..3361479d139d 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2639,13 +2639,20 @@ static void fbcon_modechanged(struct fb_info *info)
 	vc = vc_cons[par->currcon].d;
 	if (vc->vc_mode != KD_TEXT ||
 	    fbcon_info_from_console(par->currcon) != info)
 		return;
 
+	clear_selection();
+
 	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);
+	}
+
 	if (con_is_visible(vc)) {
 		var_to_display(p, &info->var, info);
 		cols = FBCON_SWAP(par->rotate, info->var.xres, info->var.yres);
 		rows = FBCON_SWAP(par->rotate, info->var.yres, info->var.xres);
 		cols /= vc->vc_font.width;
@@ -2673,10 +2680,12 @@ static void fbcon_set_all_vcs(struct fb_info *info)
 	int i, rows, cols, fg = -1;
 
 	if (!par || par->currcon < 0)
 		return;
 
+	clear_selection();
+
 	for (i = first_fb_vc; i <= last_fb_vc; i++) {
 		vc = vc_cons[i].d;
 		if (!vc || vc->vc_mode != KD_TEXT ||
 		    fbcon_info_from_console(i) != info)
 			continue;
-- 
2.52.0

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

[sashiko-bot]

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