[PATCH] fbdev: Delay the setting of fbcon_ops to fix KASAN issues

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

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;

-- 
2.39.2

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

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

在 2025/9/22 14:31, Thomas Zimmermann 写道:

Hi

Am 05.09.25 um 04:43 schrieb Zizhi Wo:

quoted

[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

quoted

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

quoted

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;