linux-kernel - Re: [PATCH v2] fbcon: Fix a NULL pointer dereference issue in fbcon

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1b1a2d3c-ed4a-4d9b-b87a-8d05f3d6592e@gmx.de>
Date: Tue, 24 Sep 2024 18:59:22 +0200
From: Helge Deller <deller@....de>
To: Qianqiang Liu <qianqiang.liu@....com>
Cc: linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org,
 syzbot+3d613ae53c031502687a@...kaller.appspotmail.com
Subject: Re: [PATCH v2] fbcon: Fix a NULL pointer dereference issue in
 fbcon_putcs

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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3d613ae53c031502687a
> Tested-by: syzbot+3d613ae53c031502687a@...kaller.appspotmail.com
> Signed-off-by: Qianqiang Liu <qianqiang.liu@....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;