Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] fbcon: UAF in panic handler due to dangling par->rotated.buf --
commit e32477b2f73cd204764bac2654d00076bb8bd3ba Author: Yao Yiqi <[email protected]> fbcon: Rebuild rotated font buffer on font change fbcon_rotate_font() caches a rotated copy of the console font in par->rotated.buf. The buffer is sized from the font's width, height and charcount, and is rebuilt in fbcon_init(), fbcon_switch() and fbcon_modechanged(), but not in fbcon_do_set_font(). > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 01715873ea49b..d90a2ad45053c 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2432,6 +2432,17 @@ static int fbcon_do_set_font(struct vc_data *vc, int > w, int h, int charcount, > else if (!vc->vc_hi_font_mask && charcount == 512) > set_vc_hi_font(vc, true); > > + /* > + * Select the console's blitting state and update par->rotated to > + * match the new font, so that the rendering below does not use a > + * cache that was built for the old font. > + */ > + if (con_is_visible(vc)) { > + set_blitting_type(vc, info); > + if (par->bitops->rotate_font) > + par->bitops->rotate_font(info, vc); [Severity: High] Does this code expose a race condition with console_flush_on_panic() that could lead to a use-after-free? When fbcon_rotate_font() is called, it passes par->rotated.buf to font_data_rotate(): drivers/video/fbdev/core/fbcon_rotate.c:fbcon_rotate_font() { ... buf = font_data_rotate(par->rotated.fontdata, vc->vc_font.width, vc->vc_font.height, vc->vc_font.charcount, par->rotated.buf_rotate, par->rotated.buf, &par->rotated.bufsize); ... } If a reallocation is needed, font_data_rotate() frees the old buffer before returning the new one: lib/fonts/font_rotate.c:font_data_rotate() { ... kfree(buf); buf = dst; ... } During this time, the global par->rotated.buf pointer remains dangling. If a kernel panic occurs in this window, console_flush_on_panic() could bypass the console lock and invoke fbcon write operations. Rendering operations like ccw_putcs() check the pointer: drivers/video/fbdev/core/fbcon_ccw.c:ccw_putcs() { ... if (!par->rotated.buf) return; ... } Because par->rotated.buf hasn't been set to NULL or updated to the new buffer yet, this check would evaluate to false, leading to a read from freed memory which could abort the panic handler and lose important debug logs. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
