Skip to content

Commit a14ca25

Browse files
maciej-w-rozyckigregkh
authored andcommitted
vt: Fix character height handling with VT_RESIZEX
commit 860dafa upstream. Restore the original intent of the VT_RESIZEX ioctl's `v_clin' parameter which is the number of pixel rows per character (cell) rather than the height of the font used. For framebuffer devices the two values are always the same, because the former is inferred from the latter one. For VGA used as a true text mode device these two parameters are independent from each other: the number of pixel rows per character is set in the CRT controller, while font height is in fact hardwired to 32 pixel rows and fonts of heights below that value are handled by padding their data with blanks when loaded to hardware for use by the character generator. One can change the setting in the CRT controller and it will update the screen contents accordingly regardless of the font loaded. The `v_clin' parameter is used by the `vgacon' driver to set the height of the character cell and then the cursor position within. Make the parameter explicit then, by defining a new `vc_cell_height' struct member of `vc_data', set it instead of `vc_font.height' from `v_clin' in the VT_RESIZEX ioctl, and then use it throughout the `vgacon' driver except where actual font data is accessed which as noted above is independent from the CRTC setting. This way the framebuffer console driver is free to ignore the `v_clin' parameter as irrelevant, as it always should have, avoiding any issues attempts to give the parameter a meaning there could have caused, such as one that has led to commit 988d076 ("vt_ioctl: make VT_RESIZEX behave like VT_RESIZE"): "syzbot is reporting UAF/OOB read at bit_putcs()/soft_cursor() [1][2], for vt_resizex() from ioctl(VT_RESIZEX) allows setting font height larger than actual font height calculated by con_font_set() from ioctl(PIO_FONT). Since fbcon_set_font() from con_font_set() allocates minimal amount of memory based on actual font height calculated by con_font_set(), use of vt_resizex() can cause UAF/OOB read for font data." The problem first appeared around Linux 2.5.66 which predates our repo history, but the origin could be identified with the old MIPS/Linux repo also at: <git://git.kernel.org/pub/scm/linux/kernel/git/ralf/linux.git> as commit 9736a35 ("Merge with Linux 2.5.66."), where VT_RESIZEX code in `vt_ioctl' was updated as follows: if (clin) - video_font_height = clin; + vc->vc_font.height = clin; making the parameter apply to framebuffer devices as well, perhaps due to the use of "font" in the name of the original `video_font_height' variable. Use "cell" in the new struct member then to avoid ambiguity. References: [1] https://syzkaller.appspot.com/bug?id=32577e96d88447ded2d3b76d71254fb855245837 [2] https://syzkaller.appspot.com/bug?id=6b8355d27b2b94fb5cedf4655e3a59162d9e48e3 Signed-off-by: Maciej W. Rozycki <[email protected]> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: [email protected] # v2.6.12+ Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 8026eb8 commit a14ca25

File tree

3 files changed

+26
-25
lines changed

3 files changed

+26
-25
lines changed

drivers/tty/vt/vt_ioctl.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -806,17 +806,17 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
806806
if (vcp) {
807807
int ret;
808808
int save_scan_lines = vcp->vc_scan_lines;
809-
int save_font_height = vcp->vc_font.height;
809+
int save_cell_height = vcp->vc_cell_height;
810810

811811
if (v.v_vlin)
812812
vcp->vc_scan_lines = v.v_vlin;
813813
if (v.v_clin)
814-
vcp->vc_font.height = v.v_clin;
814+
vcp->vc_cell_height = v.v_clin;
815815
vcp->vc_resize_user = 1;
816816
ret = vc_resize(vcp, v.v_cols, v.v_rows);
817817
if (ret) {
818818
vcp->vc_scan_lines = save_scan_lines;
819-
vcp->vc_font.height = save_font_height;
819+
vcp->vc_cell_height = save_cell_height;
820820
console_unlock();
821821
return ret;
822822
}

drivers/video/console/vgacon.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ static void vgacon_init(struct vc_data *c, int init)
384384
vc_resize(c, vga_video_num_columns, vga_video_num_lines);
385385

386386
c->vc_scan_lines = vga_scan_lines;
387-
c->vc_font.height = vga_video_font_height;
387+
c->vc_font.height = c->vc_cell_height = vga_video_font_height;
388388
c->vc_complement_mask = 0x7700;
389389
if (vga_512_chars)
390390
c->vc_hi_font_mask = 0x0800;
@@ -519,32 +519,32 @@ static void vgacon_cursor(struct vc_data *c, int mode)
519519
switch (CUR_SIZE(c->vc_cursor_type)) {
520520
case CUR_UNDERLINE:
521521
vgacon_set_cursor_size(c->state.x,
522-
c->vc_font.height -
523-
(c->vc_font.height <
522+
c->vc_cell_height -
523+
(c->vc_cell_height <
524524
10 ? 2 : 3),
525-
c->vc_font.height -
526-
(c->vc_font.height <
525+
c->vc_cell_height -
526+
(c->vc_cell_height <
527527
10 ? 1 : 2));
528528
break;
529529
case CUR_TWO_THIRDS:
530530
vgacon_set_cursor_size(c->state.x,
531-
c->vc_font.height / 3,
532-
c->vc_font.height -
533-
(c->vc_font.height <
531+
c->vc_cell_height / 3,
532+
c->vc_cell_height -
533+
(c->vc_cell_height <
534534
10 ? 1 : 2));
535535
break;
536536
case CUR_LOWER_THIRD:
537537
vgacon_set_cursor_size(c->state.x,
538-
(c->vc_font.height * 2) / 3,
539-
c->vc_font.height -
540-
(c->vc_font.height <
538+
(c->vc_cell_height * 2) / 3,
539+
c->vc_cell_height -
540+
(c->vc_cell_height <
541541
10 ? 1 : 2));
542542
break;
543543
case CUR_LOWER_HALF:
544544
vgacon_set_cursor_size(c->state.x,
545-
c->vc_font.height / 2,
546-
c->vc_font.height -
547-
(c->vc_font.height <
545+
c->vc_cell_height / 2,
546+
c->vc_cell_height -
547+
(c->vc_cell_height <
548548
10 ? 1 : 2));
549549
break;
550550
case CUR_NONE:
@@ -555,7 +555,7 @@ static void vgacon_cursor(struct vc_data *c, int mode)
555555
break;
556556
default:
557557
vgacon_set_cursor_size(c->state.x, 1,
558-
c->vc_font.height);
558+
c->vc_cell_height);
559559
break;
560560
}
561561
break;
@@ -566,13 +566,13 @@ static int vgacon_doresize(struct vc_data *c,
566566
unsigned int width, unsigned int height)
567567
{
568568
unsigned long flags;
569-
unsigned int scanlines = height * c->vc_font.height;
569+
unsigned int scanlines = height * c->vc_cell_height;
570570
u8 scanlines_lo = 0, r7 = 0, vsync_end = 0, mode, max_scan;
571571

572572
raw_spin_lock_irqsave(&vga_lock, flags);
573573

574574
vgacon_xres = width * VGA_FONTWIDTH;
575-
vgacon_yres = height * c->vc_font.height;
575+
vgacon_yres = height * c->vc_cell_height;
576576
if (vga_video_type >= VIDEO_TYPE_VGAC) {
577577
outb_p(VGA_CRTC_MAX_SCAN, vga_video_port_reg);
578578
max_scan = inb_p(vga_video_port_val);
@@ -627,9 +627,9 @@ static int vgacon_doresize(struct vc_data *c,
627627
static int vgacon_switch(struct vc_data *c)
628628
{
629629
int x = c->vc_cols * VGA_FONTWIDTH;
630-
int y = c->vc_rows * c->vc_font.height;
630+
int y = c->vc_rows * c->vc_cell_height;
631631
int rows = screen_info.orig_video_lines * vga_default_font_height/
632-
c->vc_font.height;
632+
c->vc_cell_height;
633633
/*
634634
* We need to save screen size here as it's the only way
635635
* we can spot the screen has been resized and we need to
@@ -1060,7 +1060,7 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
10601060
cursor_size_lastto = 0;
10611061
c->vc_sw->con_cursor(c, CM_DRAW);
10621062
}
1063-
c->vc_font.height = fontheight;
1063+
c->vc_font.height = c->vc_cell_height = fontheight;
10641064
vc_resize(c, 0, rows); /* Adjust console size */
10651065
}
10661066
}
@@ -1115,12 +1115,12 @@ static int vgacon_resize(struct vc_data *c, unsigned int width,
11151115
*/
11161116
screen_info.orig_video_cols = width;
11171117
screen_info.orig_video_lines = height;
1118-
vga_default_font_height = c->vc_font.height;
1118+
vga_default_font_height = c->vc_cell_height;
11191119
return 0;
11201120
}
11211121
if (width % 2 || width > screen_info.orig_video_cols ||
11221122
height > (screen_info.orig_video_lines * vga_default_font_height)/
1123-
c->vc_font.height)
1123+
c->vc_cell_height)
11241124
return -EINVAL;
11251125

11261126
if (con_is_visible(c) && !vga_is_gfx) /* who knows */

include/linux/console_struct.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ struct vc_data {
101101
unsigned int vc_rows;
102102
unsigned int vc_size_row; /* Bytes per row */
103103
unsigned int vc_scan_lines; /* # of scan lines */
104+
unsigned int vc_cell_height; /* CRTC character cell height */
104105
unsigned long vc_origin; /* [!] Start of real screen */
105106
unsigned long vc_scr_end; /* [!] End of real screen */
106107
unsigned long vc_visible_origin; /* [!] Top of visible window */

0 commit comments

Comments
 (0)