Skip to content

Commit 7115cb1

Browse files
Sergei Trofimovichgregkh
authored andcommitted
tty/vt: fix write/write race in ioctl(KDSKBSENT) handler
commit 46ca3f7 upstream. 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 <f3> 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 <gregkh@linuxfoundation.org> CC: Jiri Slaby <jslaby@suse.com> Link: https://lkml.org/lkml/2019/2/17/256 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d10dc1e commit 7115cb1

1 file changed

Lines changed: 27 additions & 6 deletions

File tree

drivers/tty/vt/keyboard.c

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ static const int NR_TYPES = ARRAY_SIZE(max_vals);
122122
static struct input_handler kbd_handler;
123123
static DEFINE_SPINLOCK(kbd_event_lock);
124124
static DEFINE_SPINLOCK(led_lock);
125+
static DEFINE_SPINLOCK(func_buf_lock); /* guard 'func_buf' and friends */
125126
static unsigned long key_down[BITS_TO_LONGS(KEY_CNT)]; /* keyboard key bitmap */
126127
static unsigned char shift_down[NR_SHIFT]; /* shift state counters.. */
127128
static bool dead_key_next;
@@ -1959,11 +1960,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
19591960
char *p;
19601961
u_char *q;
19611962
u_char __user *up;
1962-
int sz;
1963+
int sz, fnw_sz;
19631964
int delta;
19641965
char *first_free, *fj, *fnw;
19651966
int i, j, k;
19661967
int ret;
1968+
unsigned long flags;
19671969

19681970
if (!capable(CAP_SYS_TTY_CONFIG))
19691971
perm = 0;
@@ -2006,18 +2008,27 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
20062008
goto reterr;
20072009
}
20082010

2011+
fnw = NULL;
2012+
fnw_sz = 0;
2013+
/* race aginst other writers */
2014+
again:
2015+
spin_lock_irqsave(&func_buf_lock, flags);
20092016
q = func_table[i];
2017+
2018+
/* fj pointer to next entry after 'q' */
20102019
first_free = funcbufptr + (funcbufsize - funcbufleft);
20112020
for (j = i+1; j < MAX_NR_FUNC && !func_table[j]; j++)
20122021
;
20132022
if (j < MAX_NR_FUNC)
20142023
fj = func_table[j];
20152024
else
20162025
fj = first_free;
2017-
2026+
/* buffer usage increase by new entry */
20182027
delta = (q ? -strlen(q) : 1) + strlen(kbs->kb_string);
2028+
20192029
if (delta <= funcbufleft) { /* it fits in current buf */
20202030
if (j < MAX_NR_FUNC) {
2031+
/* make enough space for new entry at 'fj' */
20212032
memmove(fj + delta, fj, first_free - fj);
20222033
for (k = j; k < MAX_NR_FUNC; k++)
20232034
if (func_table[k])
@@ -2030,20 +2041,28 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
20302041
sz = 256;
20312042
while (sz < funcbufsize - funcbufleft + delta)
20322043
sz <<= 1;
2033-
fnw = kmalloc(sz, GFP_KERNEL);
2034-
if(!fnw) {
2035-
ret = -ENOMEM;
2036-
goto reterr;
2044+
if (fnw_sz != sz) {
2045+
spin_unlock_irqrestore(&func_buf_lock, flags);
2046+
kfree(fnw);
2047+
fnw = kmalloc(sz, GFP_KERNEL);
2048+
fnw_sz = sz;
2049+
if (!fnw) {
2050+
ret = -ENOMEM;
2051+
goto reterr;
2052+
}
2053+
goto again;
20372054
}
20382055

20392056
if (!q)
20402057
func_table[i] = fj;
2058+
/* copy data before insertion point to new location */
20412059
if (fj > funcbufptr)
20422060
memmove(fnw, funcbufptr, fj - funcbufptr);
20432061
for (k = 0; k < j; k++)
20442062
if (func_table[k])
20452063
func_table[k] = fnw + (func_table[k] - funcbufptr);
20462064

2065+
/* copy data after insertion point to new location */
20472066
if (first_free > fj) {
20482067
memmove(fnw + (fj - funcbufptr) + delta, fj, first_free - fj);
20492068
for (k = j; k < MAX_NR_FUNC; k++)
@@ -2056,7 +2075,9 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
20562075
funcbufleft = funcbufleft - delta + sz - funcbufsize;
20572076
funcbufsize = sz;
20582077
}
2078+
/* finally insert item itself */
20592079
strcpy(func_table[i], kbs->kb_string);
2080+
spin_unlock_irqrestore(&func_buf_lock, flags);
20602081
break;
20612082
}
20622083
ret = 0;

0 commit comments

Comments
 (0)