[PATCH v2] fbdev: Add bounds checking in bit_putcs to fix vmalloc-out-of-bounds

[PATCH v2] fbdev: Add bounds checking in bit_putcs to fix vmalloc-out-of-bounds

[Albin Babu Varghese]

Add bounds checking to prevent writes past framebuffer boundaries when
rendering text near screen edges. Return early if the Y position is off-screen
and clip image height to screen boundary. Break from the rendering loop if the
X position is off-screen. When clipping image width to fit the screen, update
the character count to match the clipped width to prevent buffer size
mismatches.

Without the character count update, bit_putcs_aligned and bit_putcs_unaligned
receive mismatched parameters where the buffer is allocated for the clipped
width but cnt reflects the original larger count, causing out-of-bounds writes.

Reported-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=48b0652a95834717f190  
Suggested-by: Helge Deller <deller@gmx.de>
Tested-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
---
Changes in v2:
- Partially render when height exceeding screen boundaries instead of skipping 
- Update character count when width is clipped to prevent buffer mismatch

Link to v1: 
https://lore.kernel.org/all/20250927075010.119671-1-albinbabuvarghese20@gmail.com/
---
 drivers/video/fbdev/core/bitblit.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index f9475c14f733..116b4bf5e94c 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -160,6 +160,11 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
 	image.height = vc->vc_font.height;
 	image.depth = 1;
 
+	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 +178,21 @@ static void bit_putcs(struct vc_data *vc, struct fb_info *info,
 			cnt = count;
 
 		image.width = vc->vc_font.width * cnt;
+
+		if (image.dx >= info->var.xres)
+			break;
+
+		if (image.dx + image.width > info->var.xres) {
+			image.width = info->var.xres - image.dx;
+
+			cnt = image.width / vc->vc_font.width;
+
+			if (cnt == 0)
+				break;
+
+			image.width = cnt * vc->vc_font.width;
+		}
+
 		pitch = DIV_ROUND_UP(image.width, 8) + scan_align;
 		pitch &= ~scan_align;
 		size = pitch * image.height + buf_align;
-- 
2.51.0

Re: [PATCH v2] fbdev: Add bounds checking in bit_putcs to fix vmalloc-out-of-bounds

[Helge Deller]

On 10/3/25 09:32, Albin Babu Varghese wrote:
> Add bounds checking to prevent writes past framebuffer boundaries when
> rendering text near screen edges. Return early if the Y position is off-screen
> and clip image height to screen boundary. Break from the rendering loop if the
> X position is off-screen. When clipping image width to fit the screen, update
> the character count to match the clipped width to prevent buffer size
> mismatches.
> 
> Without the character count update, bit_putcs_aligned and bit_putcs_unaligned
> receive mismatched parameters where the buffer is allocated for the clipped
> width but cnt reflects the original larger count, causing out-of-bounds writes.
> 
> Reported-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=48b0652a95834717f190
> Suggested-by: Helge Deller <deller@gmx.de>
> Tested-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> ---
> Changes in v2:
> - Partially render when height exceeding screen boundaries instead of skipping
> - Update character count when width is clipped to prevent buffer mismatch
> 
> Link to v1:
> https://lore.kernel.org/all/20250927075010.119671-1-albinbabuvarghese20@gmail.com/
> ---
>   drivers/video/fbdev/core/bitblit.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)

applied.

Thanks!
Helge

Re: [PATCH v2] fbdev: Add bounds checking in bit_putcs to fix vmalloc-out-of-bounds

[Albin Babu Varghese]

On Sat, Oct 04, 2025 at 02:43:33AM +0200, Helge Deller wrote:
> On 10/3/25 09:32, Albin Babu Varghese wrote:
> > Add bounds checking to prevent writes past framebuffer boundaries when
> > rendering text near screen edges. Return early if the Y position is off-screen
> > and clip image height to screen boundary. Break from the rendering loop if the
> > X position is off-screen. When clipping image width to fit the screen, update
> > the character count to match the clipped width to prevent buffer size
> > mismatches.
> > 
> > Without the character count update, bit_putcs_aligned and bit_putcs_unaligned
> > receive mismatched parameters where the buffer is allocated for the clipped
> > width but cnt reflects the original larger count, causing out-of-bounds writes.
> > 
> > Reported-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=48b0652a95834717f190
> > Suggested-by: Helge Deller <deller@gmx.de>
> > Tested-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
> > Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > ---
> > Changes in v2:
> > - Partially render when height exceeding screen boundaries instead of skipping
> > - Update character count when width is clipped to prevent buffer mismatch
> > 
> > Link to v1:
> > https://lore.kernel.org/all/20250927075010.119671-1-albinbabuvarghese20@gmail.com/
> > ---
> >   drivers/video/fbdev/core/bitblit.c | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> 
> applied.
> 
> Thanks!
> Helge
Thank you for merging the patch.

After the patch appeared in mainline, I observed that syzbot continues
to find the same bug through different execution paths. My fix addressed
bit_putcs, but the crashes now occur through bit_cursor and cw_putcs,
which bypass bit_putcs entirely and go directly to sys_imageblit():

Crash 1 (cursor path):
https://syzkaller.appspot.com/text?tag=CrashReport&x=11fe95e2580000
Call trace: hide_cursor → bit_cursor → soft_cursor → sys_imageblit

Crash 2 (rotation path):
https://syzkaller.appspot.com/text?tag=CrashReport&x=164f0b04580000
Call trace: fbcon_modechanged → cw_putcs → sys_imageblit

The original syzbot reproducer depended on character height going out of
bounds, so I focused on bit_putcs where character images are drawn. I
naively overlooked cursor drawing - apologies for that. That's why I
looked at the other crash reports after the merge, because it seemed odd
that it was still hitting the bug after the fix.

I believe adding the same clipping logic in sys_imageblit() would provide
comprehensive protection. Something like this:

void sys_imageblit(struct fb_info *p, const struct fb_image *image)
  {
 +       struct fb_image clipped;
 +       u32 width, height;
 +
         if (!(p->flags & FBINFO_VIRTFB))
                 fb_warn_once(p, "%s: framebuffer is not in virtual address space.\n", __func__);
 
 -       fb_imageblit(p, image);
 +       if (!image || image->dx >= p->var.xres || image->dy >= p->var.yres)
 +               return;
 +
 +       if (image->dx + image->width > p->var.xres || image->dy + image->height > p->var.yres) {
 +               clipped = *image;
 +
 +               height = clipped.height;
 +               if (clipped.dy + height > p->var.yres)
 +                       height = p->var.yres - clipped.dy;
 +
 +               clipped.height = height;
 +
 +               width = clipped.width;
 +               if (clipped.dx + width > p->var.xres)
 +                       width = p->var.xres - clipped.dx;
 +
 +               clipped.width = width;
 +
 +               if (clipped.width == 0 || clipped.height == 0)
 +                       return;
 +
 +               fb_imageblit(p, &clipped);
 +       } else {
 +               fb_imageblit(p, image);
 +       }
  }
 
I don't have any way to test this other than testing it with the existing
reproducers. I can prepare a patch if this approach sounds reasonable.

Thanks,
Albin

Re: [PATCH v2] fbdev: Add bounds checking in bit_putcs to fix vmalloc-out-of-bounds

[Thomas Zimmermann]

Hi

Am 04.10.25 um 02:43 schrieb Helge Deller:
> On 10/3/25 09:32, Albin Babu Varghese wrote:
>> Add bounds checking to prevent writes past framebuffer boundaries when
>> rendering text near screen edges. Return early if the Y position is 
>> off-screen
>> and clip image height to screen boundary. Break from the rendering 
>> loop if the
>> X position is off-screen. When clipping image width to fit the 
>> screen, update
>> the character count to match the clipped width to prevent buffer size
>> mismatches.
>>
>> Without the character count update, bit_putcs_aligned and 
>> bit_putcs_unaligned
>> receive mismatched parameters where the buffer is allocated for the 
>> clipped
>> width but cnt reflects the original larger count, causing 
>> out-of-bounds writes.
>>
>> Reported-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=48b0652a95834717f190
>> Suggested-by: Helge Deller <deller@gmx.de>
>> Tested-by: syzbot+48b0652a95834717f190@syzkaller.appspotmail.com
>> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
>> ---
>> Changes in v2:
>> - Partially render when height exceeding screen boundaries instead of 
>> skipping
>> - Update character count when width is clipped to prevent buffer 
>> mismatch
>>
>> Link to v1:
>> https://lore.kernel.org/all/20250927075010.119671-1-albinbabuvarghese20@gmail.com/ 
>>
>> ---
>>   drivers/video/fbdev/core/bitblit.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>
> applied.

This patch mangles fbcon output on fbdev HW drivers. It's reproducible 
on matroxfb and sisfb. Reverting the change restores these driver's 
functionality.

Fbcon from within DRM's fbdev emulation works.

Best regards
Thomas


>
> Thanks!
> Helge
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)