Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] vgacon: CUR_DEF bypasses pre-VGA hardware cursor hiding workaround --
commit d1a44aa19f5cbfffe54990d3cf6f73ab9b42b5ed Author: Thomas Zimmermann <[email protected]> vgacon: Use vt_cursor_{start,end}() Replace vgacon's custom cursor metrics with the shared helpers vc_cursor_start() and vc_font_cursor_end(). Note that the _end() function returns the index of the scanline below the cursor, while VGA hardware expects the cursor's final scanline. Hence vgacon subtracts 1 from the end value. The cursor shapes remain mostly unchanged, except that non-underline cursors now also use the glyph's top-most and bottom-most scanline. This follows the style used by fbcon. v2: - restore use of vc_cell_height (Sashiko) Signed-off-by: Thomas Zimmermann <[email protected]> Reviewed-by: Helge Deller <[email protected]> > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index 536e7fe4d1426..b1408bb5e2656 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c [ ... ] > @@ -513,10 +514,11 @@ static void vgacon_cursor(struct vc_data *c, bool > enable) > vgacon_restore_screen(c); > > c_height = c->vc_cell_height; > + c_size = CUR_SIZE(c->vc_cursor_type); > > write_vga(14, (c->vc_pos - vga_vram_base) / 2); > > - if (!enable) { > + if (!enable || c_size == CUR_NONE) { [Severity: Medium] Does this check fail to intercept CUR_DEF if the default cursor is configured as CUR_NONE? If cur_default is set to CUR_NONE and an unprivileged user triggers a cursor reset (\e[?0c), c_size will equal CUR_DEF. Since CUR_DEF does not equal CUR_NONE, this bypasses the hardware-specific hide sequence below. > if (vga_video_type >= VIDEO_TYPE_VGAC) > vgacon_set_cursor_size(31, 30); > else > @@ -524,33 +526,8 @@ static void vgacon_cursor(struct vc_data *c, bool enable) > return; > } > [ ... ] > - default: > - vgacon_set_cursor_size(1, c_height); > - break; > - } > + vgacon_set_cursor_size(vc_cursor_start(c_height, c_size), > + vc_cursor_end(c_height, c_size) - 1); [Severity: Medium] If c_size is CUR_DEF and the default cursor is CUR_NONE, could this cause cursor wrapping on EGA/MDA hardware? When vc_cursor_start() and vc_cursor_end() resolve the CUR_DEF to CUR_NONE internally, they both return c_height. This results in vgacon_set_cursor_size() being called with start = c_height and end = c_height - 1. Pre-VGA hardware (EGA/MDA/CGA 6845 CRTCs) do not support start > end for hiding cursors, which leads to a massive wrapping block cursor instead of hiding it. > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
