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

[Helge Deller <deller@gmx.de>]

Hi Qianqiang,

On 9/24/24 18:13, Qianqiang Liu wrote:
> syzbot has found a NULL pointer dereference bug in fbcon.
>
> This issue is caused by ops->putcs being a NULL pointer.
> We need to ensure it is initialized properly.
>
> Reported-by: syzbot+3d613ae53c031502687a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3d613ae53c031502687a
> Tested-by: syzbot+3d613ae53c031502687a@syzkaller.appspotmail.com
> Signed-off-by: Qianqiang Liu <qianqiang.liu@163.com>
> ---
>   Changes since v1:
>   - Initialize ops->putcs by calling set_blitting_type()

Thanks a lot tracking this issue down!

At first sight your patch seems correct.
But could you please document in the patch description what exactly (and why)
something goes wrong and how your patch fixes it?
E.g. why was opt->putcs missed to be initialized even earlier and why does
it need initialization now?

You did a good work in producing a reduced testcase.
If it's quite small, it's a good idea to even include it in the
commit message?

Helge

> ---
>   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;