linux-kernel - Re: [PATCH] fbdev: Add bounds checking in bit

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] [day] [month] [year] [list]

Message-ID: <cb00a5e2-6e50-4b01-bcd7-33eeae57ed63@gmx.de>
Date: Tue, 30 Sep 2025 22:46:23 +0200
From: Helge Deller <deller@....de>
To: Albin Babu Varghese <albinbabuvarghese20@...il.com>,
 Simona Vetter <simona@...ll.ch>
Cc: syzbot+48b0652a95834717f190@...kaller.appspotmail.com,
 linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fbdev: Add bounds checking in bit_putcs to fix
 vmalloc-out-of-bounds

On 9/27/25 09:50, Albin Babu Varghese wrote:
> KASAN reports vmalloc-out-of-bounds writes in sys_imageblit during console
> resize operations. The crash happens when bit_putcs renders characters
> outside the allocated framebuffer region.
> 
> Call trace: vc_do_resize -> clear_selection -> invert_screen ->
> do_update_region -> fbcon_putcs -> bit_putcs -> sys_imageblit
> 
> The console resize changes dimensions but bit_putcs doesn't validate that
> the character positions fit within the framebuffer before rendering.
> This causes writes past the allocated buffer in fb_imageblit functions.
> 
> Fix by checking bounds before rendering:
> - Return if dy + height > yres (would write past bottom)
> - Break if dx + width > xres (would write past right edge)
> 
> Reported-by: syzbot+48b0652a95834717f190@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=48b0652a95834717f190
> Tested-by: syzbot+48b0652a95834717f190@...kaller.appspotmail.com
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@...il.com>
> ---
>   drivers/video/fbdev/core/bitblit.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
> index f9475c14f733..4c732284384a 100644
> --- a/drivers/video/fbdev/core/bitblit.c
> +++ b/drivers/video/fbdev/core/bitblit.c
> @@ -160,6 +160,9 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
>   	image.height = vc->vc_font.height;
>   	image.depth = 1;
>   
> +	if (image.dy + image.height > info->var.yres)
> +		return;
> +

I wonder if the image.height value should be capped in this case,
instead of not rendering any chars at all?
Something like (untested!):

+	if (image.dy >= info->var.yres)
+		return;
+       image.height = min(image.height, info->var.yres - image.dy);


>   	if (attribute) {
>   		buf = kmalloc(cellsize, GFP_ATOMIC);
>   		if (!buf)
> @@ -173,6 +176,10 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
>   			cnt = count;
>   
>   		image.width = vc->vc_font.width * cnt;
> +
> +		if (image.dx + image.width > info->var.xres)
> +			break;
> +

same here.


>   		pitch = DIV_ROUND_UP(image.width, 8) + scan_align;
>   		pitch &= ~scan_align;
>   		size = pitch * image.height + buf_align;