Re: [PATCH v3] fbcon: Fix a NULL pointer dereference issue in fbcon_putcs

[Helge Deller <deller@xxxxxx>]

On 9/25/24 07:29, Qianqiang Liu wrote:
> syzbot has found a NULL pointer dereference bug in fbcon.
> Here is the simplified C reproducer:
>
> struct param {
> 	uint8_t type;
> 	struct tiocl_selection ts;
> };
>
> int main()
> {
> 	struct fb_con2fbmap con2fb;
> 	struct param param;
>
> 	int fd = open("/dev/fb1", 0, 0);
>
> 	con2fb.console = 0x19;
> 	con2fb.framebuffer = 0;
> 	ioctl(fd, FBIOPUT_CON2FBMAP, &con2fb);
>
> 	param.type = 2;
> 	param.ts.xs = 0; param.ts.ys = 0;
> 	param.ts.xe = 0; param.ts.ye = 0;
> 	param.ts.sel_mode = 0;
>
> 	int fd1 = open("/dev/tty1", O_RDWR, 0);
> 	ioctl(fd1, TIOCLINUX, &param);
>
> 	con2fb.console = 1;
> 	con2fb.framebuffer = 0;
> 	ioctl(fd, FBIOPUT_CON2FBMAP, &con2fb);
>
> 	return 0;
> }
>
> After calling ioctl(fd1, TIOCLINUX, &param), the subsequent ioctl(fd, FBIOPUT_CON2FBMAP, &con2fb)
> causes the kernel to follow a different execution path:
>
>   set_con2fb_map
>    -> con2fb_init_display
>     -> fbcon_set_disp
>      -> redraw_screen
>       -> hide_cursor
>        -> clear_selection
>         -> highlight
>          -> invert_screen
>           -> do_update_region
>            -> fbcon_putcs
>             -> ops->putcs
>
> Since ops->putcs is a NULL pointer, this leads to a kernel panic.
> To prevent this, we need to call set_blitting_type() within set_con2fb_map()
> to properly initialize ops->putcs.
>
> Reported-by: syzbot+3d613ae53c031502687a@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=3d613ae53c031502687a
> Tested-by: syzbot+3d613ae53c031502687a@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Qianqiang Liu <qianqiang.liu@xxxxxxx>
> ---
>   Changes since v2:
>   - Document the commit message in more detail

Queued up in for-next branch of fbdev git tree.

Thanks!
Helge


> ---
>   Changes since v1:
>   - Initialize ops->putcs by calling set_blitting_type()
> ---
>   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 2e093535884b..d9abae2516d8 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -861,6 +861,8 @@ static int set_con2fb_map(int unit, int newidx, int user)
>   			return err;
>
>   		fbcon_add_cursor_work(info);
> +	} else if (vc) {
> +		set_blitting_type(vc, info);
>   	}
>
>   	con2fb_map[unit] = newidx;