kernel/linux.git/drivers/tty/vt, branch v4.14.263

tty: Fix out-of-bound vmalloc access in imageblit

2021-10-06T13:05:08+00:00

[ Upstream commit 3b0c406124719b625b1aba431659f5cdc24a982c ] This issue happens when a userspace program does an ioctl FBIOPUT_VSCREENINFO passing the fb_var_screeninfo struct containing only the fields xres, yres, and bits_per_pixel with values. If this struct is the same as the previous ioctl, the vc_resize() detects it and doesn't call the resize_screen(), leaving the fb_var_screeninfo incomplete. And this leads to the updatescrollmode() calculates a wrong value to fbcon_display->vrows, which makes the real_y() return a wrong value of y, and that value, eventually, causes the imageblit to access an out-of-bound address value. To solve this issue I made the resize_screen() be called even if the screen does not need any resizing, so it will "fix and fill" the fb_var_screeninfo independently. Cc: stable # after 5.15-rc2 is out, give it time to bake Reported-and-tested-by: syzbot+858dc7a2f7ef07c2c219@syzkaller.appspotmail.com Signed-off-by: Igor Matheus Andrade Torrente Link: https://lore.kernel.org/r/20210628134509.15895-1-igormtorrente@gmail.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin

vt_kdsetmode: extend console locking

2021-09-03T07:56:26+00:00

commit 2287a51ba822384834dafc1c798453375d1107c7 upstream. As per the long-suffering comment. Reported-by: Minh Yuan Cc: Greg Kroah-Hartman Cc: Jiri Slaby Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman

tty: vt: always invoke vc->vc_sw->con_resize callback

2021-05-26T09:47:01+00:00

commit ffb324e6f874121f7dce5bdae5e05d02baae7269 upstream. syzbot is reporting OOB write at vga16fb_imageblit() [1], for resize_screen() from ioctl(VT_RESIZE) returns 0 without checking whether requested rows/columns fit the amount of memory reserved for the graphical screen if current mode is KD_GRAPHICS. ---------- #include #include #include #include #include #include int main(int argc, char *argv[]) { const int fd = open("/dev/char/4:1", O_RDWR); struct vt_sizes vt = { 0x4100, 2 }; ioctl(fd, KDSETMODE, KD_GRAPHICS); ioctl(fd, VT_RESIZE, &vt); ioctl(fd, KDSETMODE, KD_TEXT); return 0; } ---------- Allow framebuffer drivers to return -EINVAL, by moving vc->vc_mode != KD_GRAPHICS check from resize_screen() to fbcon_resize(). Link: https://syzkaller.appspot.com/bug?extid=1f29e126cf461c4de3b3 [1] Reported-by: syzbot Suggested-by: Linus Torvalds Signed-off-by: Tetsuo Handa Tested-by: syzbot Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman

vt: Fix character height handling with VT_RESIZEX

2021-05-26T09:47:01+00:00

commit 860dafa902595fb5f1d23bbcce1215188c3341e6 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 988d0763361b ("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: as commit 9736a3546de7 ("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 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org # v2.6.12+ Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman

vt/consolemap: do font sum unsigned

2021-03-07T10:27:44+00:00

[ Upstream commit 9777f8e60e718f7b022a94f2524f967d8def1931 ] The constant 20 makes the font sum computation signed which can lead to sign extensions and signed wraps. It's not much of a problem as we build with -fno-strict-overflow. But if we ever decide not to, be ready, so switch the constant to unsigned. Signed-off-by: Jiri Slaby Link: https://lore.kernel.org/r/20210105120239.28031-7-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin

vt: Disable KD_FONT_OP_COPY

2020-11-10T09:29:06+00:00

commit 3c4e0dff2095c579b142d5a0693257f1c58b4804 upstream. It's buggy: On Fri, Nov 06, 2020 at 10:30:08PM +0800, Minh Yuan wrote: > We recently discovered a slab-out-of-bounds read in fbcon in the latest > kernel ( v5.10-rc2 for now ). The root cause of this vulnerability is that > "fbcon_do_set_font" did not handle "vc->vc_font.data" and > "vc->vc_font.height" correctly, and the patch > for VT_RESIZEX can't handle this > issue. > > Specifically, we use KD_FONT_OP_SET to set a small font.data for tty6, and > use KD_FONT_OP_SET again to set a large font.height for tty1. After that, > we use KD_FONT_OP_COPY to assign tty6's vc_font.data to tty1's vc_font.data > in "fbcon_do_set_font", while tty1 retains the original larger > height. Obviously, this will cause an out-of-bounds read, because we can > access a smaller vc_font.data with a larger vc_font.height. Further there was only one user ever. - Android's loadfont, busybox and console-tools only ever use OP_GET and OP_SET - fbset documentation only mentions the kernel cmdline font: option, not anything else. - systemd used OP_COPY before release 232 published in Nov 2016 Now unfortunately the crucial report seems to have gone down with gmane, and the commit message doesn't say much. But the pull request hints at OP_COPY being broken https://github.com/systemd/systemd/pull/3651 So in other words, this never worked, and the only project which foolishly every tried to use it, realized that rather quickly too. Instead of trying to fix security issues here on dead code by adding missing checks, fix the entire thing by removing the functionality. Note that systemd code using the OP_COPY function ignored the return value, so it doesn't matter what we're doing here really - just in case a lone server somewhere happens to be extremely unlucky and running an affected old version of systemd. The relevant code from font_copy_to_all_vcs() in systemd was: /* copy font from active VT, where the font was uploaded to */ cfo.op = KD_FONT_OP_COPY; cfo.height = vcs.v_active-1; /* tty1 == index 0 */ (void) ioctl(vcfd, KDFONTOP, &cfo); Note this just disables the ioctl, garbage collecting the now unused callbacks is left for -next. v2: Tetsuo found the old mail, which allowed me to find it on another archive. Add the link too. Acked-by: Peilin Ye Reported-by: Minh Yuan References: https://lists.freedesktop.org/archives/systemd-devel/2016-June/036935.html References: https://github.com/systemd/systemd/pull/3651 Cc: Greg KH Cc: Peilin Ye Cc: Tetsuo Handa Signed-off-by: Daniel Vetter Link: https://lore.kernel.org/r/20201108153806.3140315-1-daniel.vetter@ffwll.ch Signed-off-by: Greg Kroah-Hartman

tty: make FONTX ioctl use the tty pointer they were actually passed

2020-11-05T10:07:04+00:00

commit 90bfdeef83f1d6c696039b6a917190dcbbad3220 upstream. Some of the font tty ioctl's always used the current foreground VC for their operations. Don't do that then. This fixes a data race on fg_console. Side note: both Michael Ellerman and Jiri Slaby point out that all these ioctls are deprecated, and should probably have been removed long ago, and everything seems to be using the KDFONTOP ioctl instead. In fact, Michael points out that it looks like busybox's loadfont program seems to have switched over to using KDFONTOP exactly _because_ of this bug (ahem.. 12 years ago ;-). Reported-by: Minh Yuan Acked-by: Michael Ellerman Acked-by: Jiri Slaby Cc: Greg KH Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman

vt: keyboard, extend func_buf_lock to readers

2020-11-05T10:07:01+00:00

commit 82e61c3909db51d91b9d3e2071557b6435018b80 upstream. Both read-side users of func_table/func_buf need locking. Without that, one can easily confuse the code by repeatedly setting altering strings like: while (1) for (a = 0; a < 2; a++) { struct kbsentry kbs = {}; strcpy((char *)kbs.kb_string, a ? ".\n" : "88888\n"); ioctl(fd, KDSKBSENT, &kbs); } When that program runs, one can get unexpected output by holding F1 (note the unxpected period on the last line): . 88888 .8888 So protect all accesses to 'func_table' (and func_buf) by preexisting 'func_buf_lock'. It is easy in 'k_fn' handler as 'puts_queue' is expected not to sleep. On the other hand, KDGKBSENT needs a local (atomic) copy of the string because copy_to_user can sleep. Use already allocated, but unused 'kbs->kb_string' for that purpose. Note that the program above needs at least CAP_SYS_TTY_CONFIG. This depends on the previous patch and on the func_buf_lock lock added in commit 46ca3f735f34 (tty/vt: fix write/write race in ioctl(KDSKBSENT) handler) in 5.2. Likely fixes CVE-2020-25656. Cc: Reported-by: Minh Yuan Signed-off-by: Jiri Slaby Link: https://lore.kernel.org/r/20201019085517.10176-2-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman

vt: keyboard, simplify vt_kdgkbsent

2020-11-05T10:07:01+00:00

commit 6ca03f90527e499dd5e32d6522909e2ad390896b upstream. Use 'strlen' of the string, add one for NUL terminator and simply do 'copy_to_user' instead of the explicit 'for' loop. This makes the KDGKBSENT case more compact. The only thing we need to take care about is NULL 'func_table[i]'. Use an empty string in that case. The original check for overflow could never trigger as the func_buf strings are always shorter or equal to 'struct kbsentry's. Cc: Signed-off-by: Jiri Slaby Link: https://lore.kernel.org/r/20201019085517.10176-1-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman

vt_ioctl: change VT_RESIZEX ioctl to check for error return from vc_resize()

2020-09-03T09:22:31+00:00

commit bc5269ca765057a1b762e79a1cfd267cd7bf1c46 upstream. vc_resize() can return with an error after failure. Change VT_RESIZEX ioctl to save struct vc_data values that are modified and restore the original values in case of error. Signed-off-by: George Kennedy Cc: stable Reported-by: syzbot+38a3699c7eaf165b97a6@syzkaller.appspotmail.com Link: https://lore.kernel.org/r/1596213192-6635-2-git-send-email-george.kennedy@oracle.com Signed-off-by: Greg Kroah-Hartman