linux-kernel - Re: [PATCH] fbdev: Delay the setting of fbcon

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: <97658279-73a4-4d30-817b-6dcd47a11d6b@suse.de>
Date: Mon, 22 Sep 2025 08:31:06 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Zizhi Wo <wozizhi@...weicloud.com>, deller@....de, lee@...nel.org,
 jani.nikula@...el.com, oushixiong@...inos.cn, soci@....rulez.org
Cc: linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, yangerkun@...wei.com
Subject: Re: [PATCH] fbdev: Delay the setting of fbcon_ops to fix KASAN issues

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

>
> 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 <wozizhi@...weicloud.com>
> ---
>   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)