在 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 <[email protected]>
---
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;
