Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Nov 18, 2022 at 12:27:58AM +0900, Tetsuo Handa wrote:
> A kernel built with syzbot's config file reported that
> 
>   scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2))
> 
> causes uninitialized "save" to be copied.
> 
>   ----------
>   [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 0
>   [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 1
>   Console: switching to colour frame buffer device 128x48
>   =====================================================
>   BUG: KMSAN: uninit-value in do_update_region+0x4b8/0xba0
>    do_update_region+0x4b8/0xba0
>    update_region+0x40d/0x840
>    fbcon_switch+0x3364/0x35e0
>    redraw_screen+0xae3/0x18a0
>    do_bind_con_driver+0x1cb3/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   Uninit was stored to memory at:
>    fbcon_prepare_logo+0x143b/0x1940
>    fbcon_init+0x2c1b/0x31c0
>    visual_init+0x3e7/0x820
>    do_bind_con_driver+0x14a4/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   Uninit was created at:
>    __kmem_cache_alloc_node+0xb69/0x1020
>    __kmalloc+0x379/0x680
>    fbcon_prepare_logo+0x704/0x1940
>    fbcon_init+0x2c1b/0x31c0
>    visual_init+0x3e7/0x820
>    do_bind_con_driver+0x14a4/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc4-00356-g8f2975c2bb4c #924
>   Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>   ----------
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks for your patch, pushed to drm-misc-fixes.
-Daniel

> ---
>  drivers/video/fbdev/core/fbcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 098b62f7b701..c0143d38df83 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -577,7 +577,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
>  		if (scr_readw(r) != vc->vc_video_erase_char)
>  			break;
>  	if (r != q && new_rows >= rows + logo_lines) {
> -		save = kmalloc(array3_size(logo_lines, new_cols, 2),
> +		save = kzalloc(array3_size(logo_lines, new_cols, 2),
>  			       GFP_KERNEL);
>  		if (save) {
>  			int i = min(cols, new_cols);
> -- 
> 2.34.1

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch