Re: [RFC PATCH] tty: vt: Fix vc_origin buffer copy overflow in fbcon_prepare_logo()

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 25.09.25 um 11:15 schrieb Wang ShaoBo:
> I got some KASAN report as below:
>
> BUG: KASAN: slab-use-after-free in fbcon_prepare_logo+0x61e/0xc90
> Read of size 14 at addr ffff88812c9a4c38 by task syz.0.3549/19016
>
> CPU: 0 PID: 19016 Comm: syz.0.3549 Not tainted 6.6.0+ #80
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> Call Trace:
>   <TASK>
>   dump_stack_lvl+0x72/0xa0
>   print_address_description.constprop.0+0x6b/0x3d0
>   ? fbcon_prepare_logo+0x61e/0xc90
>   print_report+0xba/0x280
>   ? fbcon_prepare_logo+0x61e/0xc90
>   ? kasan_addr_to_slab+0xd/0xa0
>   ? fbcon_prepare_logo+0x61e/0xc90
>   kasan_report+0xaf/0xe0
>   ? fbcon_prepare_logo+0x61e/0xc90
>   kasan_check_range+0x100/0x1c0
>   __asan_memcpy+0x23/0x60
>   fbcon_prepare_logo+0x61e/0xc90
>   fbcon_init+0xeb9/0x1db0
>   ? __pfx_drm_fb_helper_set_par+0x10/0x10
>   visual_init+0x310/0x5c0
>   do_bind_con_driver.isra.0+0x627/0xbd0
>   store_bind+0x60b/0x710
>   ? __pfx_store_bind+0x10/0x10
>   dev_attr_store+0x5a/0x90
>   ? __pfx_dev_attr_store+0x10/0x10
>   sysfs_kf_write+0x145/0x1b0
>   kernfs_fop_write_iter+0x367/0x580
>   ? __pfx_sysfs_kf_write+0x10/0x10
>   new_sync_write+0x1b1/0x2d0
>   ? __pfx_new_sync_write+0x10/0x10
>   ? rb_commit+0x121/0x910
>   ? avc_policy_seqno+0xe/0x20
>   ? selinux_file_permission+0x129/0x5d0
>   ? security_file_permission+0xa8/0x700
>   vfs_write+0x71a/0x960
>   ksys_write+0x12e/0x260
>
> fbcon_init()
> 	-> vc_resize()
> 	//success resize vc_origin buffer size=224=7(cols)*2*16(rows)
> 	-> bcon_prepare_logo(vc, info, old_cols, old_rows,
>                              new_cols, new_rows)
> 	//old_cols=256，old_rows=4，new_cols=7，new_rows=16
>
> There happened to be a vc_origin buffer copy overflow error in
> fbcon_prepare_logo(), scrolling screen down when using old cols
> after vc resize would trigger out of lower bound of vc_origin buffer.
>
> Fixes: 6104c37094e7 ("fbcon: Make fbcon a built-time depency for fbdev")

That's probably not the broken commit. According to 'git blame', the 
broken code dates back to the initial git import. During the pre-git 
days, the change at [1] might be have introduced he issue.

Maybe drop the Fixes tag entirely.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=458a55138545b8b098feb29aaf2e509f8fbab4b3

> Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com>

This looks like a sensible change to me, although I can't claim to fully 
understand what the function does.

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> ---
>   drivers/video/fbdev/core/fbcon.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index ac3c99ed92d1..6fa81d0297a0 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -610,6 +610,8 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
>   		/* We can scroll screen down */
>   		r = q - step - cols;
>   		for (cnt = rows - logo_lines; cnt > 0; cnt--) {
> +			if (r < (unsigned short *) vc->vc_origin)
> +				break;
>   			scr_memcpyw(r + step, r, vc->vc_size_row);
>   			r -= cols;
>   		}

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)