Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] fbcon: dangling pointer in par->rotated.fontdata causes ABA vulnerability - [High] fbcon: OOB read and UAF in fbcon_do_set_font error path --
commit 40ac874a29ca63690bb8c7dfaad9a195bb52891d 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. This commit fixes a slab-out-of-bounds read in fbcon when changing the console font by explicitly rebuilding the rotated font buffer before the redraw is triggered. > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 615fa596b7cce..caad4e06944ea 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -2431,6 +2431,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); > > + /* > + * The rotated font buffer is sized from the new font's width, height > + * and charcount. Rebuild it for the visible console before the redraw > + * below, as fbcon_switch() and fbcon_modechanged() already do. The > + * redraw reaches the rotated bitops through hide_cursor(), and those > + * index the buffer with the new cell size and character codes, so a > + * stale, too-small buffer is read out of bounds. > + */ > + if (con_is_visible(vc) && par->bitops->rotate_font) > + par->bitops->rotate_font(info, vc); > + [Severity: High] If vc_resize() fails below this point (e.g., due to -ENOMEM from screen buffer allocation), the err_out path reverts the console font metrics but leaves the rotated buffer and pointer out of sync. Could this leave par->rotated.buf sized for the new font, while the font metrics are restored to the old font? If a subsequent call uses a smaller font, the buffer could be allocated exactly to the smaller size. If vc_resize() then fails, the metrics are restored to the larger font, which could cause a slab-out-of-bounds read during subsequent redraws (such as cursor blink). Furthermore, the err_out path drops the new font's refcount via font_data_put(data), and the memory is subsequently freed by the caller. Does this leave par->rotated.fontdata as a dangling pointer to this freed memory? If a future font allocation reuses this exact slab address, the ABA logic in fbcon_rotate_font(): if (par->p->fontdata == par->rotated.fontdata && par->rotate == par->rotated.buf_rotate) return 0; would incorrectly assume the font hasn't changed and skip rebuilding the buffer, perpetuating the metric mismatch and leading to further out-of-bounds accesses. > if (resize) { > int cols, rows; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
