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

On 7/29/26 04:12, Zizhi Wo wrote:
> From: Zizhi Wo <[email protected]>
> 
> [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 <[email protected]>
> ---
> v3:
> Add comments.
> 
> v2:
> Fixed the issue by calling clear_selection() earlier, and updated the
> related description in the commit message.
> https://lore.kernel.org/all/[email protected]/
> 
> v1:
> https://lore.kernel.org/all/[email protected]/
> ---
> ---
>   drivers/video/fbdev/core/fbcon.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
patch applied.

Thanks!
Helge