--- /dev/null
+From 82e61c3909db51d91b9d3e2071557b6435018b80 Mon Sep 17 00:00:00 2001
+From: Jiri Slaby <jirislaby@kernel.org>
+Date: Mon, 19 Oct 2020 10:55:17 +0200
+Subject: vt: keyboard, extend func_buf_lock to readers
+
+From: Jiri Slaby <jslaby@suse.cz>
+
+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: <stable@vger.kernel.org>
+Reported-by: Minh Yuan <yuanmingbuaa@gmail.com>
+Signed-off-by: Jiri Slaby <jslaby@suse.cz>
+Link: https://lore.kernel.org/r/20201019085517.10176-2-jslaby@suse.cz
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ drivers/tty/vt/keyboard.c | 17 +++++++++++++----
+ 1 file changed, 13 insertions(+), 4 deletions(-)
+
+--- a/drivers/tty/vt/keyboard.c
++++ b/drivers/tty/vt/keyboard.c
+@@ -712,8 +712,13 @@ static void k_fn(struct vc_data *vc, uns
+ return;
+
+ if ((unsigned)value < ARRAY_SIZE(func_table)) {
++ unsigned long flags;
++
++ spin_lock_irqsave(&func_buf_lock, flags);
+ if (func_table[value])
+ puts_queue(vc, func_table[value]);
++ spin_unlock_irqrestore(&func_buf_lock, flags);
++
+ } else
+ pr_err("k_fn called with value=%d\n", value);
+ }
+@@ -1969,7 +1974,7 @@ out:
+ #undef s
+ #undef v
+
+-/* FIXME: This one needs untangling and locking */
++/* FIXME: This one needs untangling */
+ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
+ {
+ struct kbsentry *kbs;
+@@ -2001,10 +2006,14 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
+ switch (cmd) {
+ case KDGKBSENT: {
+ /* size should have been a struct member */
+- unsigned char *from = func_table[i] ? : "";
++ ssize_t len = sizeof(user_kdgkb->kb_string);
++
++ spin_lock_irqsave(&func_buf_lock, flags);
++ len = strlcpy(kbs->kb_string, func_table[i] ? : "", len);
++ spin_unlock_irqrestore(&func_buf_lock, flags);
+
+- ret = copy_to_user(user_kdgkb->kb_string, from,
+- strlen(from) + 1) ? -EFAULT : 0;
++ ret = copy_to_user(user_kdgkb->kb_string, kbs->kb_string,
++ len + 1) ? -EFAULT : 0;
+
+ goto reterr;
+ }
--- /dev/null
+From 6ca03f90527e499dd5e32d6522909e2ad390896b Mon Sep 17 00:00:00 2001
+From: Jiri Slaby <jirislaby@kernel.org>
+Date: Mon, 19 Oct 2020 10:55:16 +0200
+Subject: vt: keyboard, simplify vt_kdgkbsent
+
+From: Jiri Slaby <jslaby@suse.cz>
+
+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: <stable@vger.kernel.org>
+Signed-off-by: Jiri Slaby <jslaby@suse.cz>
+Link: https://lore.kernel.org/r/20201019085517.10176-1-jslaby@suse.cz
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ drivers/tty/vt/keyboard.c | 28 +++++++++-------------------
+ 1 file changed, 9 insertions(+), 19 deletions(-)
+
+--- a/drivers/tty/vt/keyboard.c
++++ b/drivers/tty/vt/keyboard.c
+@@ -1973,9 +1973,7 @@ out:
+ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
+ {
+ struct kbsentry *kbs;
+- char *p;
+ u_char *q;
+- u_char __user *up;
+ int sz, fnw_sz;
+ int delta;
+ char *first_free, *fj, *fnw;
+@@ -2001,23 +1999,15 @@ int vt_do_kdgkb_ioctl(int cmd, struct kb
+ i = kbs->kb_func;
+
+ switch (cmd) {
+- case KDGKBSENT:
+- sz = sizeof(kbs->kb_string) - 1; /* sz should have been
+- a struct member */
+- up = user_kdgkb->kb_string;
+- p = func_table[i];
+- if(p)
+- for ( ; *p && sz; p++, sz--)
+- if (put_user(*p, up++)) {
+- ret = -EFAULT;
+- goto reterr;
+- }
+- if (put_user('\0', up)) {
+- ret = -EFAULT;
+- goto reterr;
+- }
+- kfree(kbs);
+- return ((p && *p) ? -EOVERFLOW : 0);
++ case KDGKBSENT: {
++ /* size should have been a struct member */
++ unsigned char *from = func_table[i] ? : "";
++
++ ret = copy_to_user(user_kdgkb->kb_string, from,
++ strlen(from) + 1) ? -EFAULT : 0;
++
++ goto reterr;
++ }
+ case KDSKBSENT:
+ if (!perm) {
+ ret = -EPERM;
projects / thirdparty / kernel / stable-queue.git / commitdiff
