| 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
| ||
|
Message-ID: <394c720f-d23e-4208-b1d6-e0b98b03fc91@huaweicloud.com>
Date: Mon, 22 Sep 2025 19:42:16 +0800
From: Zizhi Wo <wozizhi@...weicloud.com>
To: Thomas Zimmermann <tzimmermann@...e.de>, 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
在 2025/9/22 14:31, Thomas Zimmermann 写道:
> 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?
As far as I can remember, op.width = 32 and op.height = 12;
And I also do the TIOCL_SETSEL ioctl to set vc_sel.start && vc_sel.end
>
>> 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
Sorry, my reproduction script has been cleaned up because some time has
passed. But the root cause of the issue is still setting ops too early,
which leads to vc_resize() calling clear_selection(), then eventually
.putcs. This uses the updated rotation-related functions on the previous
region, which may cause out-of-bounds access.
If this patch series does not ensure that the old putcs is used in the
context of clear_selection() during vc_resize(), the problem may still
exist?
Thanks,
Zizhi Wo
>
> 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;
>
Powered by blists - more mailing lists