From 46ca3f735f345c9d87383dd3a09fa5d43870770e Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sun, 10 Mar 2019 21:24:15 +0000 Subject: tty/vt: fix write/write race in ioctl(KDSKBSENT) handler The bug manifests as an attempt to access deallocated memory: BUG: unable to handle kernel paging request at ffff9c8735448000 #PF error: [PROT] [WRITE] PGD 288a05067 P4D 288a05067 PUD 288a07067 PMD 7f60c2063 PTE 80000007f5448161 Oops: 0003 [#1] PREEMPT SMP CPU: 6 PID: 388 Comm: loadkeys Tainted: G C 5.0.0-rc6-00153-g5ded5871030e #91 Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./H77M-D3H, BIOS F12 11/14/2013 RIP: 0010:__memmove+0x81/0x1a0 Code: 4c 89 4f 10 4c 89 47 18 48 8d 7f 20 73 d4 48 83 c2 20 e9 a2 00 00 00 66 90 48 89 d1 4c 8b 5c 16 f8 4c 8d 54 17 f8 48 c1 e9 03 48 a5 4d 89 1a e9 0c 01 00 00 0f 1f 40 00 48 89 d1 4c 8b 1e 49 RSP: 0018:ffffa1b9002d7d08 EFLAGS: 00010203 RAX: ffff9c873541af43 RBX: ffff9c873541af43 RCX: 00000c6f105cd6bf RDX: 0000637882e986b6 RSI: ffff9c8735447ffb RDI: ffff9c8735447ffb RBP: ffff9c8739cd3800 R08: ffff9c873b802f00 R09: 00000000fffff73b R10: ffffffffb82b35f1 R11: 00505b1b004d5b1b R12: 0000000000000000 R13: ffff9c873541af3d R14: 000000000000000b R15: 000000000000000c FS: 00007f450c390580(0000) GS:ffff9c873f180000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff9c8735448000 CR3: 00000007e213c002 CR4: 00000000000606e0 Call Trace: vt_do_kdgkb_ioctl+0x34d/0x440 vt_ioctl+0xba3/0x1190 ? __bpf_prog_run32+0x39/0x60 ? mem_cgroup_commit_charge+0x7b/0x4e0 tty_ioctl+0x23f/0x920 ? preempt_count_sub+0x98/0xe0 ? __seccomp_filter+0x67/0x600 do_vfs_ioctl+0xa2/0x6a0 ? syscall_trace_enter+0x192/0x2d0 ksys_ioctl+0x3a/0x70 __x64_sys_ioctl+0x16/0x20 do_syscall_64+0x54/0xe0 entry_SYSCALL_64_after_hwframe+0x49/0xbe The bug manifests on systemd systems with multiple vtcon devices: # cat /sys/devices/virtual/vtconsole/vtcon0/name (S) dummy device # cat /sys/devices/virtual/vtconsole/vtcon1/name (M) frame buffer device There systemd runs 'loadkeys' tool in tapallel for each vtcon instance. This causes two parallel ioctl(KDSKBSENT) calls to race into adding the same entry into 'func_table' array at: drivers/tty/vt/keyboard.c:vt_do_kdgkb_ioctl() The function has no locking around writes to 'func_table'. The simplest reproducer is to have initrams with the following init on a 8-CPU machine x86_64: #!/bin/sh loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & loadkeys -q windowkeys ru4 & wait The change adds lock on write path only. Reads are still racy. CC: Greg Kroah-Hartman CC: Jiri Slaby Link: https://lkml.org/lkml/2019/2/17/256 Signed-off-by: Sergei Trofimovich Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/keyboard.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index 88312c6c92cc..0617e87ab343 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -123,6 +123,7 @@ static const int NR_TYPES = ARRAY_SIZE(max_vals); static struct input_handler kbd_handler; static DEFINE_SPINLOCK(kbd_event_lock); static DEFINE_SPINLOCK(led_lock); +static DEFINE_SPINLOCK(func_buf_lock); /* guard 'func_buf' and friends */ static unsigned long key_down[BITS_TO_LONGS(KEY_CNT)]; /* keyboard key bitmap */ static unsigned char shift_down[NR_SHIFT]; /* shift state counters.. */ static bool dead_key_next; @@ -1990,11 +1991,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) char *p; u_char *q; u_char __user *up; - int sz; + int sz, fnw_sz; int delta; char *first_free, *fj, *fnw; int i, j, k; int ret; + unsigned long flags; if (!capable(CAP_SYS_TTY_CONFIG)) perm = 0; @@ -2037,7 +2039,14 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) goto reterr; } + fnw = NULL; + fnw_sz = 0; + /* race aginst other writers */ + again: + spin_lock_irqsave(&func_buf_lock, flags); q = func_table[i]; + + /* fj pointer to next entry after 'q' */ first_free = funcbufptr + (funcbufsize - funcbufleft); for (j = i+1; j < MAX_NR_FUNC && !func_table[j]; j++) ; @@ -2045,10 +2054,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) fj = func_table[j]; else fj = first_free; - + /* buffer usage increase by new entry */ delta = (q ? -strlen(q) : 1) + strlen(kbs->kb_string); + if (delta <= funcbufleft) { /* it fits in current buf */ if (j < MAX_NR_FUNC) { + /* make enough space for new entry at 'fj' */ memmove(fj + delta, fj, first_free - fj); for (k = j; k < MAX_NR_FUNC; k++) if (func_table[k]) @@ -2061,20 +2072,28 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) sz = 256; while (sz < funcbufsize - funcbufleft + delta) sz <<= 1; - fnw = kmalloc(sz, GFP_KERNEL); - if(!fnw) { - ret = -ENOMEM; - goto reterr; + if (fnw_sz != sz) { + spin_unlock_irqrestore(&func_buf_lock, flags); + kfree(fnw); + fnw = kmalloc(sz, GFP_KERNEL); + fnw_sz = sz; + if (!fnw) { + ret = -ENOMEM; + goto reterr; + } + goto again; } if (!q) func_table[i] = fj; + /* copy data before insertion point to new location */ if (fj > funcbufptr) memmove(fnw, funcbufptr, fj - funcbufptr); for (k = 0; k < j; k++) if (func_table[k]) func_table[k] = fnw + (func_table[k] - funcbufptr); + /* copy data after insertion point to new location */ if (first_free > fj) { memmove(fnw + (fj - funcbufptr) + delta, fj, first_free - fj); for (k = j; k < MAX_NR_FUNC; k++) @@ -2087,7 +2106,9 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm) funcbufleft = funcbufleft - delta + sz - funcbufsize; funcbufsize = sz; } + /* finally insert item itself */ strcpy(func_table[i], kbs->kb_string); + spin_unlock_irqrestore(&func_buf_lock, flags); break; } ret = 0; -- cgit v1.2.3 From 75ddbc1fb11efac87b611d48e9802f6fe2bb2163 Mon Sep 17 00:00:00 2001 From: Yifeng Li Date: Tue, 5 Mar 2019 07:02:49 +0800 Subject: tty: vt.c: Fix TIOCL_BLANKSCREEN console blanking if blankinterval == 0 Previously, in the userspace, it was possible to use the "setterm" command from util-linux to blank the VT console by default, using the following command. According to the man page, > The force option keeps the screen blank even if a key is pressed. It was implemented by calling TIOCL_BLANKSCREEN. case BLANKSCREEN: ioctlarg = TIOCL_BLANKSCREEN; if (ioctl(STDIN_FILENO, TIOCLINUX, &ioctlarg)) warn(_("cannot force blank")); break; However, after Linux 4.12, this command ceased to work anymore, which is unexpected. By inspecting the kernel source, it shows that the issue was triggered by the side-effect from commit a4199f5eb809 ("tty: Disable default console blanking interval"). The console blanking is implemented by function do_blank_screen() in vt.c: "blank_state" will be initialized to "blank_normal_wait" in con_init() if AND ONLY IF ("blankinterval" > 0). If "blankinterval" is 0, "blank_state" will be "blank_off" (== 0), and a call to do_blank_screen() will always abort, even if a forced blanking is required from the user by calling TIOCL_BLANKSCREEN, the console won't be blanked. This behavior is unexpected from a user's point-of-view, since it's not mentioned in any documentation. The setterm man page suggests it will always work, and the kernel comments in uapi/linux/tiocl.h says > /* keep screen blank even if a key is pressed */ > #define TIOCL_BLANKSCREEN 14 To fix it, we simply remove the "blank_state != blank_off" check, as pointed out by Nicolas Pitre, this check doesn't logically make sense and it's safe to remove. Suggested-by: Nicolas Pitre Fixes: a4199f5eb809 ("tty: Disable default console blanking interval") Signed-off-by: Yifeng Li Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/vt.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index d34984aa646d..721edee50234 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -4178,8 +4178,6 @@ void do_blank_screen(int entering_gfx) return; } - if (blank_state != blank_normal_wait) - return; blank_state = blank_off; /* don't blank graphics */ -- cgit v1.2.3 From 67fbfc3943e03464541d2cbe8ff8ff77cfe7fe93 Mon Sep 17 00:00:00 2001 From: Jakub Wilk Date: Mon, 11 Mar 2019 10:08:45 +0100 Subject: vt: use /dev/vcs (not /dev/vcs0) in comment Both /dev/vcs and /dev/vcs0 were in use in the past, but these days /dev/vcs0 is mostly historical curiosity. * "/dev/vcs" is the name that has always been in the Linux allocated devices list. * "vcs" is the device name in sysfs since Linux v2.6.12. * MAKEDEV(1) in Debian used to create /dev/vcs0 only, but /dev/vcs was added in 1999: https://bugs.debian.org/45698 * MAKEDEV(1) in RedHat switched from /dev/vcs0 to /dev/vcs in 2000: * Fri Oct 20 2000 Nalin Dahyabhai - change vcs0 to vcs (ditto for vcsa0) Signed-off-by: Jakub Wilk Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/vc_screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c index 160f46115aaa..1f042346e722 100644 --- a/drivers/tty/vt/vc_screen.c +++ b/drivers/tty/vt/vc_screen.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* * Provide access to virtual console memory. - * /dev/vcs0: the screen as it is being viewed right now (possibly scrolled) + * /dev/vcs: the screen as it is being viewed right now (possibly scrolled) * /dev/vcsN: the screen of /dev/ttyN (1 <= N <= 63) * [minor: N] * -- cgit v1.2.3 From f16aa97d3f575ea660f49d4698efe1c4a4c60919 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 2 Apr 2019 16:07:08 +0200 Subject: tty: fix up a few remaining files without SPDX identifiers There were a few straggling files under drivers/tty/ that did not have any SPDX identifier either because they entered the tree recently, or they somehow missed the mass-tagging of commit b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license") This commit follows the same rule as b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license") where files without any specified license in them fall under GPL-2.0 as the correct license for the individual file. Add that identifier to these remaining files so that we don't have to guess at the license of them in the future. Cc: Jiri Slaby Cc: "David S. Miller" Reviewed-by: Mukesh Ojha Signed-off-by: Greg Kroah-Hartman --- drivers/tty/serial/8250/8250_men_mcb.c | 1 + drivers/tty/serial/sn_console.c | 1 + drivers/tty/vcc.c | 1 + drivers/tty/vt/.gitignore | 1 + drivers/tty/vt/cp437.uni | 1 + drivers/tty/vt/defkeymap.c_shipped | 1 + drivers/tty/vt/defkeymap.map | 1 + 7 files changed, 7 insertions(+) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/serial/8250/8250_men_mcb.c b/drivers/tty/serial/8250/8250_men_mcb.c index 127017cc41d9..02c5aff58a74 100644 --- a/drivers/tty/serial/8250/8250_men_mcb.c +++ b/drivers/tty/serial/8250/8250_men_mcb.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 #include #include #include diff --git a/drivers/tty/serial/sn_console.c b/drivers/tty/serial/sn_console.c index fe9170731c16..283493358a62 100644 --- a/drivers/tty/serial/sn_console.c +++ b/drivers/tty/serial/sn_console.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * C-Brick Serial Port (and console) driver for SGI Altix machines. * diff --git a/drivers/tty/vcc.c b/drivers/tty/vcc.c index 58b454c34560..d2a1e1228c82 100644 --- a/drivers/tty/vcc.c +++ b/drivers/tty/vcc.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* vcc.c: sun4v virtual channel concentrator * * Copyright (C) 2017 Oracle. All rights reserved. diff --git a/drivers/tty/vt/.gitignore b/drivers/tty/vt/.gitignore index 83683a2d8e6a..9b38b85f9d9a 100644 --- a/drivers/tty/vt/.gitignore +++ b/drivers/tty/vt/.gitignore @@ -1,2 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 consolemap_deftbl.c defkeymap.c diff --git a/drivers/tty/vt/cp437.uni b/drivers/tty/vt/cp437.uni index bc6163484f62..a1991904c559 100644 --- a/drivers/tty/vt/cp437.uni +++ b/drivers/tty/vt/cp437.uni @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 # # Unicode table for IBM Codepage 437. Note that there are many more # substitutions that could be conceived (for example, thick-line diff --git a/drivers/tty/vt/defkeymap.c_shipped b/drivers/tty/vt/defkeymap.c_shipped index d2208dfe3f67..c7095fb7d2d1 100644 --- a/drivers/tty/vt/defkeymap.c_shipped +++ b/drivers/tty/vt/defkeymap.c_shipped @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* Do not edit this file! It was automatically generated by */ /* loadkeys --mktable defkeymap.map > defkeymap.c */ diff --git a/drivers/tty/vt/defkeymap.map b/drivers/tty/vt/defkeymap.map index 50b30cace261..37f1ac6ddfb9 100644 --- a/drivers/tty/vt/defkeymap.map +++ b/drivers/tty/vt/defkeymap.map @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 # Default kernel keymap. This uses 7 modifier combinations. keymaps 0-2,4-5,8,12 # Change the above line into -- cgit v1.2.3 From fa2b360f261e31f2a54f997095713f91bac0e503 Mon Sep 17 00:00:00 2001 From: Konstantin Khorenko Date: Mon, 15 Apr 2019 14:17:55 +0300 Subject: tty/vt: avoid high order pages allocation on GIO_UNIMAP ioctl GIO_UNIMAP can easily result in a high order allocation, seen 6th order allocation on radeondrmfb: fbcon: radeondrmfb (fb0) is primary device Console: switching to colour frame buffer device 160x64 radeon 0000:01:05.0: fb0: radeondrmfb frame buffer device WARNING: CPU: 0 PID: 78661 at mm/page_alloc.c:3532 __alloc_pages_nodemask+0x1b1/0x600 order 6 >= 3, gfp 0x40d0 The warning is generated by a debug patch. At the same time it's safe to use kvmalloc() for allocation in con_get_unimap(), so let's do the substitution. And do the same for con_set_unimap(). Signed-off-by: Konstantin Khorenko Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/consolemap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c index 7c7ada0b3ea0..b28aa0d289f8 100644 --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -542,7 +542,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) if (!ct) return 0; - unilist = memdup_user(list, ct * sizeof(struct unipair)); + unilist = vmemdup_user(list, ct * sizeof(struct unipair)); if (IS_ERR(unilist)) return PTR_ERR(unilist); @@ -641,7 +641,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list) out_unlock: console_unlock(); - kfree(unilist); + kvfree(unilist); return err; } @@ -743,7 +743,7 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni struct uni_pagedir *p; struct unipair *unilist; - unilist = kmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); + unilist = kvmalloc_array(ct, sizeof(struct unipair), GFP_KERNEL); if (!unilist) return -ENOMEM; @@ -775,7 +775,7 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct, struct uni if (copy_to_user(list, unilist, min(ect, ct) * sizeof(struct unipair))) ret = -EFAULT; put_user(ect, uct); - kfree(unilist); + kvfree(unilist); return ret ? ret : (ect <= ct) ? 0 : -ENOMEM; } -- cgit v1.2.3 From 89bb1e1ee529d9d06ca694ba22a35dc2a3d6ac67 Mon Sep 17 00:00:00 2001 From: Reinis Danne Date: Thu, 11 Apr 2019 14:50:54 +0300 Subject: tty: vt: keyboard: Allow Unicode compose base char MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass Unicode character to k_unicode handler function to honor possible accent_table definition. With introduction of KDSKBDIACRUC ioctl, it has been possible to set compose characters (diacr, base and result) to any Unicode character (below 0xf000 code point) as defined in struct kbdiacruc. But it does not work with characters beyond Latin-1 as base, because they are printed early and not passed to any of the handler functions, thus breaking compose and dead keys. It affects keyboard layouts defining such characters on level 1 and relying on dead keys to access level 3 (e.g., lv-modern with some modifications for extra characters on level 3 for ē, ā and ī keys). Signed-off-by: Reinis Danne Signed-off-by: Greg Kroah-Hartman --- drivers/tty/vt/keyboard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/vt') diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c index 0617e87ab343..515fc095e3b4 100644 --- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c @@ -1450,7 +1450,7 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw) KBD_UNICODE, ¶m); if (rc != NOTIFY_STOP) if (down && !raw_mode) - to_utf8(vc, keysym); + k_unicode(vc, keysym, !down); return; } -- cgit v1.2.3