Re: [PATCH] fbdev: Delay the setting of fbcon_ops to fix KASAN issues
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2025-09-22 06:31:08
Also in:
dri-devel, lkml
Hi Am 05.09.25 um 04:43 schrieb Zizhi Wo:
[BUG] Recently, we encountered a KASAN warning as follows: kasan_report+0xaf/0xe0 mm/kasan/report.c:588 fb_pad_aligned_buffer+0x12f/0x150 drivers/video/fbdev/core/fbmem.c:116 ccw_putcs_aligned drivers/video/fbdev/core/fbcon_ccw.c:119 [inline] ccw_putcs+0x9ac/0xbb0 drivers/video/fbdev/core/fbcon_ccw.c:175 fbcon_putcs+0x329/0x3f0 drivers/video/fbdev/core/fbcon.c:1297 do_update_region+0x3de/0x670 drivers/tty/vt/vt.c:623 invert_screen+0x1de/0x600 drivers/tty/vt/vt.c:748 highlight drivers/tty/vt/selection.c:57 [inline] clear_selection+0x5e/0x70 drivers/tty/vt/selection.c:81 vc_do_resize+0xc8e/0xf40 drivers/tty/vt/vt.c:1206 fbcon_modechanged+0x489/0x7a0 drivers/video/fbdev/core/fbcon.c:2705 fbcon_set_all_vcs+0x1e0/0x600 drivers/video/fbdev/core/fbcon.c:2752 fbcon_rotate_all drivers/video/fbdev/core/fbcon.c:250 [inline] ... reproduce[probabilistic, depending on the width and height of vc_font, as well as the value of "p" in do_update_region()]:
Which font sizes trigger the bug?
1) echo 2 > /sys/devices/virtual/graphics/fbcon/rotate_all 2) echo 3 > /sys/devices/virtual/graphics/fbcon/rotate_all [CAUSE] The root cause is that fbcon_modechanged() first sets the current rotate's corresponding ops. Subsequently, during vc_resize(), it may trigger clear_selection(), and in fbcon_putcs->ccw_putcs[rotate=3], this can result in an out-of-bounds access to "src". This happens because ops->fontbuffer is reallocated in fbcon_rotate_font(): 1) When rotate=2, its size is (width + 7) / 8 * height 2) When rotate=3, its size is (height + 7) / 8 * width And the call to fbcon_rotate_font() occurs after clear_selection(). In other words, the fontbuffer is allocated using the size calculated from the previous rotation[2], but before reallocating it with the new size, con_putcs is already using the new rotation[3]:
We recently reworked the way rotation callbacks are set. [1] Does the bug still happen with [1] applied? [1] https://patchwork.freedesktop.org/series/153056/#rev2 Best regards Thomas
quoted hunk ↗ jump to hunk
rotate_all_store fbcon_rotate_all fbcon_set_all_vcs fbcon_modechanged ... fbcon_set_rotate fbcon_rotate_ccw ops->putcs = ccw_putcs // set rotate 3 ops vc_resize ... clear_selection highlight ... do_update_region fbcon_putcs ccw_putcs_aligned src = ops->fontbuffer + (scr_readw(s--) & charmask)*cellsize fb_pad_aligned_buffer----[src KASAN!!!] update_screen redraw_screen fbcon_switch fbcon_rotate_font dst = kmalloc_array(len, d_cellsize, GFP_KERNEL) ops->fontbuffer = dst [FIX] Considering that when the rotation changes, clear_selection() should clear the previously selected region and not consider the new rotation yet. Therefore, the assignment to fbcon_ops for the newly set rotate can be postponed to fbcon_rotate_font(), since the fontbuffer is regenerated there. To avoid affecting other code paths, fbcon_set_rotate() will temporarily continue assigning fbcon_ops based on cur_rotate not rotate. Signed-off-by: Zizhi Wo <redacted> --- drivers/video/fbdev/core/fbcon_rotate.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c index ec3c883400f7..d76446da24d4 100644 --- a/drivers/video/fbdev/core/fbcon_rotate.c +++ b/drivers/video/fbdev/core/fbcon_rotate.c@@ -70,6 +70,7 @@ static int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc) src += s_cellsize; dst += d_cellsize; } + fbcon_rotate_ud(ops); break; case FB_ROTATE_CW: for (i = len; i--; ) {@@ -78,6 +79,7 @@ static int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc) src += s_cellsize; dst += d_cellsize; } + fbcon_rotate_cw(ops); break; case FB_ROTATE_CCW: for (i = len; i--; ) {@@ -86,6 +88,7 @@ static int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc) src += s_cellsize; dst += d_cellsize; } + fbcon_rotate_ccw(ops); break; }@@ -97,7 +100,7 @@ void fbcon_set_rotate(struct fbcon_ops *ops) { ops->rotate_font = fbcon_rotate_font; - switch(ops->rotate) { + switch (ops->cur_rotate) { case FB_ROTATE_CW: fbcon_rotate_cw(ops); break;
-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)