Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] fbcon: stale rotated font cache key in fbcon_rotate_font() on error rollback --
commit 4c61e799dc92607244ca176d7070e7e3d2fde5e4 Author: Yao Yiqi <[email protected]> fbcon: Retry rotated font allocation after failure This commit checks par->rotated.buf in the early-out condition of fbcon_rotate_font() so that a failed rotated font allocation is retried on the next call instead of leaving the console blank. > diff --git a/drivers/video/fbdev/core/fbcon_rotate.c > b/drivers/video/fbdev/core/fbcon_rotate.c > index 660a3fa81baed..7afd63dcee7be 100644 > --- a/drivers/video/fbdev/core/fbcon_rotate.c > +++ b/drivers/video/fbdev/core/fbcon_rotate.c > @@ -25,7 +25,8 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data > *vc) > par->rotate == par->rotated.buf_rotate && > vc->vc_font.width == par->rotated.buf_width && > vc->vc_font.height == par->rotated.buf_height && > - vc->vc_font.charcount == par->rotated.buf_charcount) > + vc->vc_font.charcount == par->rotated.buf_charcount && > + par->rotated.buf) > return 0; > > par->rotated.fontdata = par->p->fontdata; [Severity: High] Should fbcon_rotate_font() update the cache key fields in par->rotated only after font_data_rotate() succeeds, rather than relying solely on checking par->rotated.buf in the early-out? Because fbcon_rotate_font() records par->rotated.fontdata, buf_rotate, buf_width, buf_height, and buf_charcount before calling font_data_rotate(), those fields still get overwritten with the new font's metadata when font_data_rotate() fails. In drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font(), the caller saves par->rotated.buf before calling rotate_font() and restores the old buffer on failure: drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font() { if (con_is_visible(vc)) { set_blitting_type(vc, info); if (par->bitops->rotate_font) { #ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION saved_rotated_buf = par->rotated.buf; saved_rotated_bufsize = par->rotated.bufsize; par->rotated.buf = NULL; par->rotated.bufsize = 0; rotated_saved = true; #endif ret = par->bitops->rotate_font(info, vc); if (ret) goto err_out; } } ... err_out: if (rotated_saved) { kfree(par->rotated.buf); par->rotated.buf = saved_rotated_buf; par->rotated.bufsize = saved_rotated_bufsize; } } If rotate_font() (or the subsequent vc_resize()) fails in fbcon_do_set_font() when switching a visible console from a smaller font (e.g. 8x8) to a larger font (e.g. 16x32), err_out restores par->rotated.buf to the smaller font's non-NULL saved_rotated_buf while leaving par->rotated.{fontdata, buf_rotate, buf_width, buf_height, buf_charcount} set to the failed larger font's metadata. If another console already uses that same larger font (or if changing to a built-in/shared font matching that stale key is retried), the next fbcon_switch() -> fbcon_rotate_font() sees all key fields match and par->rotated.buf != NULL, so it returns 0 without reallocating or rotating the buffer. Could the rotated rendering callbacks (cw_putcs(), ccw_putcs(), ud_putcs(), cw_cursor(), ccw_cursor(), ud_cursor()) then read out of bounds from the smaller restored par->rotated.buf using the larger font's geometry? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
