Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [Critical] Changing the console font while the console is rotated fails to update the rotated font buffer, leading to out-of-bounds memory reads. - [High] The error fallback to unrotated mode (FB_ROTATE_UR) when `fbcon_rotate_font()` fails is overridden, leaving the console in a permanently blanked state. - [High] Cached state variables in `fbcon_rotate_font()` are updated before attempting allocation, preventing recovery if the allocation fails. -- commit bb8bca6b1feaf32fb858b99e293b0b6eee16a8cd Author: HAN Ruidong <[email protected]> fbcon: Avoid OOB font access if console rotation fails This commit clears the font buffer and size if reallocation during console rotation fails in fbcon_rotate_font(). This prevents the rotated buffer implementations from overflowing when printing with a high-enough character code on an undersized old buffer. Link: https://elixir.bootlin.com/linux/v6.19/source/drivers/video/fbdev/core/fbcon_ccw.c#L144 # [1] > diff --git a/drivers/video/fbdev/core/fbcon_rotate.c > b/drivers/video/fbdev/core/fbcon_rotate.c > index ec3c883400f7b..4a06e71ae4434 100644 > --- a/drivers/video/fbdev/core/fbcon_rotate.c > +++ b/drivers/video/fbdev/core/fbcon_rotate.c > @@ -46,6 +46,10 @@ static int fbcon_rotate_font(struct fb_info *info, struct > vc_data *vc) > info->fbops->fb_sync(info); > > if (ops->fd_size < d_cellsize * len) { > + kfree(ops->fontbuffer); > + ops->fontbuffer = NULL; > + ops->fd_size = 0; > + > dst = kmalloc_array(len, d_cellsize, GFP_KERNEL); > > if (dst == NULL) { [Severity: High] This is a pre-existing issue, but updating the cached state before the allocation in fbcon_rotate_font() seems to prevent recovery if the allocation fails. Earlier in this function, the state is updated unconditionally: src = ops->fontdata = vc->vc_font.data; ops->cur_rotate = ops->p->con_rotate; If kmalloc_array() fails here and returns -ENOMEM, the state variables are not reverted. If a subsequent retry occurs with the same font parameters, the cache check at the top of the function evaluates to true: if (vc->vc_font.data == ops->fontdata && ops->p->con_rotate == ops->cur_rotate) goto finished; Will this bypass the allocation logic and return success prematurely, leaving ops->fontbuffer as NULL indefinitely and the console blanked? [Severity: Critical] This is a pre-existing issue, but is there a missing call to fbcon_rotate_font() when changing the console font while the console is rotated? In drivers/video/fbdev/core/fbcon.c:fbcon_do_set_font(), the unrotated font state is updated: vc->vc_font.width = w; vc->vc_font.height = h; vc->vc_font.charcount = charcount; This triggers a screen redraw via update_screen(), but fbcon_rotate_font() does not appear to be called to regenerate the rotated font buffer. When update_screen() invokes the rotated putcs functions, they compute index offsets using the new font dimensions. Could this lead to reading out of bounds from the stale, potentially undersized ops->fontbuffer if the new font has a larger character count or dimensions? [Severity: High] This isn't a bug introduced by this patch, but does the fallback to unrotated mode actually work when fbcon_rotate_font() fails? If fbcon_rotate_font() returns an error, the callers (fbcon_init() and fbcon_switch() in drivers/video/fbdev/core/fbcon.c) attempt to disable rotation: if (ops->rotate_font && ops->rotate_font(info, vc)) { ops->rotate = FB_ROTATE_UR; set_blitting_type(vc, info); } However, set_blitting_type() internally calls fbcon_set_rotation(): fbcon.c:fbcon_set_rotation() { ... if (!(info->flags & FBINFO_MISC_TILEBLITTING) && ops->p->con_rotate < 4) ops->rotate = ops->p->con_rotate; else ops->rotate = 0; } Does this unconditionally overwrite ops->rotate back to the requested rotation angle (ops->p->con_rotate)? If the fallback is reverted, the rotated bitops are installed but ops->fontbuffer is NULL, which causes them to abort and leaves the console permanently blanked. > @@ -54,7 +58,6 @@ static int fbcon_rotate_font(struct fb_info *info, struct > vc_data *vc) > } > > ops->fd_size = d_cellsize * len; > - kfree(ops->fontbuffer); > ops->fontbuffer = dst; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
