[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()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The bounds checking for glyph indices is missing in software rotation paths (fbcon_ccw.c, fbcon_cw.c, fbcon_ud.c), leaving them vulnerable to out-of-bounds heap reads.
--

--- Patch [1]: [PATCH] fbdev: bitblit: bound-check glyph index in bit_cursor() ---
commit eb8c6b0049962d4bc836a2579e035e53104adf59
Author: Rik van Riel <riel@surriel.com>

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

This commit adds bounds checking for the glyph index in bit_cursor() to prevent
out-of-bounds reads when fetching glyphs. It addresses cases where the screen
buffer contains values larger than the current font's glyph count, which can
happen after a font resize or via malicious writes to /dev/vcsa.

Link: https://lore.kernel.org/all/6a75205c.01d0871a.3a0d52.0032.GAE@google.com/
Closes: https://syzkaller.appspot.com/bug?extid=61b1db46218109869c14

> diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
> index 65681dcc5930d..39f44258d793f 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));

[Severity: High]
This isn't a bug introduced by this patch, but while checking the bounds for
the glyph index in bitblit.c is a great fix, could similar out-of-bounds
reads still occur in the software rotation paths?

Looking at the rotation code (fbcon_ccw.c, fbcon_cw.c, fbcon_ud.c), the bounds
checking appears to be missing entirely. For example, in fbcon_ccw.c:

drivers/video/fbdev/core/fbcon_ccw.c:ccw_cursor() {
    ...
    c = scr_readw((u16 *) vc->vc_pos);
    attribute = get_attribute(info, c);
    src = par->rotated.buf + ((c & charmask) * (w * vc->vc_font.width));
    ...
}

And in the putcs equivalents:

drivers/video/fbdev/core/fbcon_ccw.c:ccw_putcs_aligned() {
    ...
    src = par->rotated.buf + (scr_readw(s--) & charmask) * cellsize;
    ...
}

If a user loads a custom font with fewer than 256 or 512 glyphs (e.g., 100
glyphs), the rotation buffer par->rotated.buf is sized dynamically based on
charcount. If they then write an arbitrary 16-bit character index (e.g., 255)
to the screen buffer via /dev/vcsa using vcs_write(), these rotated blit
functions will mask the index with 0xff or 0x1ff and directly use it to index
par->rotated.buf. 

Without clamping against vc->vc_font.charcount, wouldn't this result in
reading past the end of the heap-allocated rotation buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807221956.4e9dde5c@fangorn?part=1

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