[PATCH] fbdev: bitblit: bound-check glyph index in bit_cursor()

[PATCH] fbdev: bitblit: bound-check glyph index in bit_cursor()

[Rik van Riel]

bit_cursor() fetches the glyph under the cursor with

	c = scr_readw(vc_pos);
	src = vc_font.data + ((c & charmask) * w * height);

where charmask is 0x1ff when vc_hi_font_mask is set. The screen buffer
value comes directly from scr_readw() and may be larger than the current
font's glyph count.

Syzkaller triggers this via vcs_write(). The Call Trace shows
vcs_write() in vc_screen.c writing an arbitrary 16-bit value with
writev() to /dev/vcsa, which vcs_write_buf() in vc_screen.c stores via
vcs_scr_writew() without checking charcount. The stored value is later
read in bit_cursor() in bitblit.c.

When the font is changed from a font with 512 glyphs to a font with
256 glyphs, the screen buffer can retain characters with the high
bit set from the previous mode, which could also produce the same 
out-of-bounds access.

  BUG: KASAN: global-out-of-bounds in soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70
  Read of size 16 at addr ffff800086c57970

  Call Trace:
   soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70
   bit_cursor+0xa90/0x1108 drivers/video/fbdev/core/bitblit.c:365
   fbcon_cursor+0x344/0x498 drivers/video/fbdev/core/fbcon.c:1427
   hide_cursor+0xdc/0x2d0 drivers/tty/vt/vt.c:883
   update_region+0x100/0x18c drivers/tty/vt/vt.c:669
   vcs_write+0x8ec/0xaf0 drivers/tty/vt/vc_screen.c:685

bit_putcs_aligned() and bit_putcs_unaligned() already clamp the glyph
index to vc_font.charcount. Apply the same clamp in bit_cursor() after
extracting the attribute and masking, before indexing fontdata.

The fix completes the bounds checking started in commit 18c4ef4e765a
("fbdev: bitblit: bound-check glyph index in bit_putcs*"), which missed
the cursor path.

This change should be safe because the clamp reuses the existing
contract from fbcon: charcount is maintained under console_lock in
con_font_set() and fbcon_font_set(), and hi_font_mask is cleared when
switching from 512 to 256 glyphs. When stale screen data with high bits
remains after a font switch, or when vcs_write() stores an arbitrary
value, clamping the index to 0 prevents the out-of-bounds read without
changing cursor semantics — the same fallback bit_putcs uses.

Reported-by: syzbot+61b1db46218109869c14@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=61b1db46218109869c14
Link: https://lore.kernel.org/all/6a75205c.01d0871a.3a0d52.0032.GAE@google.com/
Fixes: 18c4ef4e765a ("fbdev: bitblit: bound-check glyph index in bit_putcs*")
Cc: stable@vger.kernel.org
Assisted-by: Hermes:muse-spark-1.2 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
---
 drivers/video/fbdev/core/bitblit.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 65681dcc5930..39f44258d793 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -273,9 +273,14 @@ static void bit_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
 	if (!vc->vc_font.data)
 		return;
 
- 	c = scr_readw((u16 *) vc->vc_pos);
+	c = scr_readw((u16 *) vc->vc_pos);
 	attribute = get_attribute(info, c);
-	src = vc->vc_font.data + ((c & charmask) * (w * vc->vc_font.height));
+	c &= charmask;
+
+	/* Clamp to font size, same as bit_putcs_aligned() */
+	if (c >= vc->vc_font.charcount)
+		c = 0;
+	src = vc->vc_font.data + (c * (w * vc->vc_font.height));
 
 	if (par->cursor_state.image.data != (const char *)src ||
 	    par->cursor_reset) {
-- 
2.55.0

Re: [PATCH] fbdev: bitblit: bound-check glyph index in bit_cursor()

[Helge Deller]

On 8/8/26 04:19, Rik van Riel wrote:
> bit_cursor() fetches the glyph under the cursor with
> 
> 	c = scr_readw(vc_pos);
> 	src = vc_font.data + ((c & charmask) * w * height);
> 
> where charmask is 0x1ff when vc_hi_font_mask is set. The screen buffer
> value comes directly from scr_readw() and may be larger than the current
> font's glyph count.
> 
> Syzkaller triggers this via vcs_write(). The Call Trace shows
> vcs_write() in vc_screen.c writing an arbitrary 16-bit value with
> writev() to /dev/vcsa, which vcs_write_buf() in vc_screen.c stores via
> vcs_scr_writew() without checking charcount. The stored value is later
> read in bit_cursor() in bitblit.c.
> 
> When the font is changed from a font with 512 glyphs to a font with
> 256 glyphs, the screen buffer can retain characters with the high
> bit set from the previous mode, which could also produce the same
> out-of-bounds access.
> 
>    BUG: KASAN: global-out-of-bounds in soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70
>    Read of size 16 at addr ffff800086c57970
> 
>    Call Trace:
>     soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70
>     bit_cursor+0xa90/0x1108 drivers/video/fbdev/core/bitblit.c:365
>     fbcon_cursor+0x344/0x498 drivers/video/fbdev/core/fbcon.c:1427
>     hide_cursor+0xdc/0x2d0 drivers/tty/vt/vt.c:883
>     update_region+0x100/0x18c drivers/tty/vt/vt.c:669
>     vcs_write+0x8ec/0xaf0 drivers/tty/vt/vc_screen.c:685
> 
> bit_putcs_aligned() and bit_putcs_unaligned() already clamp the glyph
> index to vc_font.charcount. Apply the same clamp in bit_cursor() after
> extracting the attribute and masking, before indexing fontdata.
> 
> The fix completes the bounds checking started in commit 18c4ef4e765a
> ("fbdev: bitblit: bound-check glyph index in bit_putcs*"), which missed
> the cursor path.
> 
> This change should be safe because the clamp reuses the existing
> contract from fbcon: charcount is maintained under console_lock in
> con_font_set() and fbcon_font_set(), and hi_font_mask is cleared when
> switching from 512 to 256 glyphs. When stale screen data with high bits
> remains after a font switch, or when vcs_write() stores an arbitrary
> value, clamping the index to 0 prevents the out-of-bounds read without
> changing cursor semantics — the same fallback bit_putcs uses.
> 
> Reported-by: syzbot+61b1db46218109869c14@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=61b1db46218109869c14
> Link: https://lore.kernel.org/all/6a75205c.01d0871a.3a0d52.0032.GAE@google.com/
> Fixes: 18c4ef4e765a ("fbdev: bitblit: bound-check glyph index in bit_putcs*")
> Cc: stable@vger.kernel.org
> Assisted-by: Hermes:muse-spark-1.2 syzkaller
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>   drivers/video/fbdev/core/bitblit.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
Patch applied to fbdev git tree.
Thanks!
Helge