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

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 25.09.25 um 16:24 schrieb Thomas Zimmermann:
> 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)