queue-4.4/tty-vt-fix-write-write-race-in-ioctl-kdskbsent-handler.patch

   1 From 46ca3f735f345c9d87383dd3a09fa5d43870770e Mon Sep 17 00:00:00 2001
   2 From: Sergei Trofimovich <slyfox@gentoo.org>
   3 Date: Sun, 10 Mar 2019 21:24:15 +0000
   4 Subject: tty/vt: fix write/write race in ioctl(KDSKBSENT) handler
   5
   6 From: Sergei Trofimovich <slyfox@gentoo.org>
   7
   8 commit 46ca3f735f345c9d87383dd3a09fa5d43870770e upstream.
   9
  10 The bug manifests as an attempt to access deallocated memory:
  11
  12     BUG: unable to handle kernel paging request at ffff9c8735448000
  13     #PF error: [PROT] [WRITE]
  14     PGD 288a05067 P4D 288a05067 PUD 288a07067 PMD 7f60c2063 PTE 80000007f5448161
  15     Oops: 0003 [#1] PREEMPT SMP
  16     CPU: 6 PID: 388 Comm: loadkeys Tainted: G         C        5.0.0-rc6-00153-g5ded5871030e #91
  17     Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./H77M-D3H, BIOS F12 11/14/2013
  18     RIP: 0010:__memmove+0x81/0x1a0
  19     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
  20     RSP: 0018:ffffa1b9002d7d08 EFLAGS: 00010203
  21     RAX: ffff9c873541af43 RBX: ffff9c873541af43 RCX: 00000c6f105cd6bf
  22     RDX: 0000637882e986b6 RSI: ffff9c8735447ffb RDI: ffff9c8735447ffb
  23     RBP: ffff9c8739cd3800 R08: ffff9c873b802f00 R09: 00000000fffff73b
  24     R10: ffffffffb82b35f1 R11: 00505b1b004d5b1b R12: 0000000000000000
  25     R13: ffff9c873541af3d R14: 000000000000000b R15: 000000000000000c
  26     FS:  00007f450c390580(0000) GS:ffff9c873f180000(0000) knlGS:0000000000000000
  27     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  28     CR2: ffff9c8735448000 CR3: 00000007e213c002 CR4: 00000000000606e0
  29     Call Trace:
  30      vt_do_kdgkb_ioctl+0x34d/0x440
  31      vt_ioctl+0xba3/0x1190
  32      ? __bpf_prog_run32+0x39/0x60
  33      ? mem_cgroup_commit_charge+0x7b/0x4e0
  34      tty_ioctl+0x23f/0x920
  35      ? preempt_count_sub+0x98/0xe0
  36      ? __seccomp_filter+0x67/0x600
  37      do_vfs_ioctl+0xa2/0x6a0
  38      ? syscall_trace_enter+0x192/0x2d0
  39      ksys_ioctl+0x3a/0x70
  40      __x64_sys_ioctl+0x16/0x20
  41      do_syscall_64+0x54/0xe0
  42      entry_SYSCALL_64_after_hwframe+0x49/0xbe
  43
  44 The bug manifests on systemd systems with multiple vtcon devices:
  45   # cat /sys/devices/virtual/vtconsole/vtcon0/name
  46   (S) dummy device
  47   # cat /sys/devices/virtual/vtconsole/vtcon1/name
  48   (M) frame buffer device
  49
  50 There systemd runs 'loadkeys' tool in tapallel for each vtcon
  51 instance. This causes two parallel ioctl(KDSKBSENT) calls to
  52 race into adding the same entry into 'func_table' array at:
  53
  54     drivers/tty/vt/keyboard.c:vt_do_kdgkb_ioctl()
  55
  56 The function has no locking around writes to 'func_table'.
  57
  58 The simplest reproducer is to have initrams with the following
  59 init on a 8-CPU machine x86_64:
  60
  61     #!/bin/sh
  62
  63     loadkeys -q windowkeys ru4 &
  64     loadkeys -q windowkeys ru4 &
  65     loadkeys -q windowkeys ru4 &
  66     loadkeys -q windowkeys ru4 &
  67
  68     loadkeys -q windowkeys ru4 &
  69     loadkeys -q windowkeys ru4 &
  70     loadkeys -q windowkeys ru4 &
  71     loadkeys -q windowkeys ru4 &
  72     wait
  73
  74 The change adds lock on write path only. Reads are still racy.
  75
  76 CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  77 CC: Jiri Slaby <jslaby@suse.com>
  78 Link: https://lkml.org/lkml/2019/2/17/256
  79 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
  80 Cc: stable <stable@vger.kernel.org>
  81 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  82 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  83
  84 ---
  85  drivers/tty/vt/keyboard.c |   33 +++++++++++++++++++++++++++------
  86  1 file changed, 27 insertions(+), 6 deletions(-)
  87
  88 --- a/drivers/tty/vt/keyboard.c
  89 +++ b/drivers/tty/vt/keyboard.c
  90 @@ -121,6 +121,7 @@ static const int NR_TYPES = ARRAY_SIZE(m
  91  static struct input_handler kbd_handler;
  92  static DEFINE_SPINLOCK(kbd_event_lock);
  93  static DEFINE_SPINLOCK(led_lock);
  94 +static DEFINE_SPINLOCK(func_buf_lock); /* guard 'func_buf'  and friends */
  95  static unsigned long key_down[BITS_TO_LONGS(KEY_CNT)]; /* keyboard key bitmap */
  96  static unsigned char shift_down[NR_SHIFT];             /* shift state counters.. */
  97  static bool dead_key_next;
  98 @@ -1969,11 +1970,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
  99         char *p;
 100         u_char *q;
 101         u_char __user *up;
 102 -       int sz;
 103 +       int sz, fnw_sz;
 104         int delta;
 105         char *first_free, *fj, *fnw;
 106         int i, j, k;
 107         int ret;
 108 +       unsigned long flags;
 109
 110         if (!capable(CAP_SYS_TTY_CONFIG))
 111                 perm = 0;
 112 @@ -2016,7 +2018,14 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
 113                         goto reterr;
 114                 }
 115
 116 +               fnw = NULL;
 117 +               fnw_sz = 0;
 118 +               /* race aginst other writers */
 119 +               again:
 120 +               spin_lock_irqsave(&func_buf_lock, flags);
 121                 q = func_table[i];
 122 +
 123 +               /* fj pointer to next entry after 'q' */
 124                 first_free = funcbufptr + (funcbufsize - funcbufleft);
 125                 for (j = i+1; j < MAX_NR_FUNC && !func_table[j]; j++)
 126                         ;
 127 @@ -2024,10 +2033,12 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
 128                         fj = func_table[j];
 129                 else
 130                         fj = first_free;
 131 -
 132 +               /* buffer usage increase by new entry */
 133                 delta = (q ? -strlen(q) : 1) + strlen(kbs->kb_string);
 134 +
 135                 if (delta <= funcbufleft) {     /* it fits in current buf */
 136                     if (j < MAX_NR_FUNC) {
 137 +                       /* make enough space for new entry at 'fj' */
 138                         memmove(fj + delta, fj, first_free - fj);
 139                         for (k = j; k < MAX_NR_FUNC; k++)
 140                             if (func_table[k])
 141 @@ -2040,20 +2051,28 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
 142                     sz = 256;
 143                     while (sz < funcbufsize - funcbufleft + delta)
 144                       sz <<= 1;
 145 -                   fnw = kmalloc(sz, GFP_KERNEL);
 146 -                   if(!fnw) {
 147 -                     ret = -ENOMEM;
 148 -                     goto reterr;
 149 +                   if (fnw_sz != sz) {
 150 +                     spin_unlock_irqrestore(&func_buf_lock, flags);
 151 +                     kfree(fnw);
 152 +                     fnw = kmalloc(sz, GFP_KERNEL);
 153 +                     fnw_sz = sz;
 154 +                     if (!fnw) {
 155 +                       ret = -ENOMEM;
 156 +                       goto reterr;
 157 +                     }
 158 +                     goto again;
 159                     }
 160
 161                     if (!q)
 162                       func_table[i] = fj;
 163 +                   /* copy data before insertion point to new location */
 164                     if (fj > funcbufptr)
 165                         memmove(fnw, funcbufptr, fj - funcbufptr);
 166                     for (k = 0; k < j; k++)
 167                       if (func_table[k])
 168                         func_table[k] = fnw + (func_table[k] - funcbufptr);
 169
 170 +                   /* copy data after insertion point to new location */
 171                     if (first_free > fj) {
 172                         memmove(fnw + (fj - funcbufptr) + delta, fj, first_free - fj);
 173                         for (k = j; k < MAX_NR_FUNC; k++)
 174 @@ -2066,7 +2085,9 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
 175                     funcbufleft = funcbufleft - delta + sz - funcbufsize;
 176                     funcbufsize = sz;
 177                 }
 178 +               /* finally insert item itself */
 179                 strcpy(func_table[i], kbs->kb_string);
 180 +               spin_unlock_irqrestore(&func_buf_lock, flags);
 181                 break;
 182         }
 183         ret = 0;