From: Greg Kroah-Hartman Date: Tue, 3 Nov 2020 14:18:47 +0000 (+0100) Subject: 4.4-stable patches X-Git-Tag: v4.14.204~33 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e4058dfeb93b502c1270177ed3b9c0e5464aadc0;p=thirdparty%2Fkernel%2Fstable-queue.git 4.4-stable patches added patches: vt-keyboard-extend-func_buf_lock-to-readers.patch vt-keyboard-simplify-vt_kdgkbsent.patch --- diff --git a/queue-4.4/series b/queue-4.4/series index 23254dd2456..fbd5ba117fa 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -41,3 +41,5 @@ acpi-cpufreq-honor-_psd-table-setting-on-new-amd-cpus.patch w1-mxc_w1-fix-timeout-resolution-problem-leading-to-bus-error.patch scsi-mptfusion-fix-null-pointer-dereferences-in-mptscsih_remove.patch btrfs-reschedule-if-necessary-when-logging-directory-items.patch +vt-keyboard-simplify-vt_kdgkbsent.patch +vt-keyboard-extend-func_buf_lock-to-readers.patch diff --git a/queue-4.4/vt-keyboard-extend-func_buf_lock-to-readers.patch b/queue-4.4/vt-keyboard-extend-func_buf_lock-to-readers.patch new file mode 100644 index 00000000000..92bd3d0fb25 --- /dev/null +++ b/queue-4.4/vt-keyboard-extend-func_buf_lock-to-readers.patch @@ -0,0 +1,94 @@ +From 82e61c3909db51d91b9d3e2071557b6435018b80 Mon Sep 17 00:00:00 2001 +From: Jiri Slaby +Date: Mon, 19 Oct 2020 10:55:17 +0200 +Subject: vt: keyboard, extend func_buf_lock to readers + +From: Jiri Slaby + +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 + +--- + 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; + } diff --git a/queue-4.4/vt-keyboard-simplify-vt_kdgkbsent.patch b/queue-4.4/vt-keyboard-simplify-vt_kdgkbsent.patch new file mode 100644 index 00000000000..cc42cd66809 --- /dev/null +++ b/queue-4.4/vt-keyboard-simplify-vt_kdgkbsent.patch @@ -0,0 +1,73 @@ +From 6ca03f90527e499dd5e32d6522909e2ad390896b Mon Sep 17 00:00:00 2001 +From: Jiri Slaby +Date: Mon, 19 Oct 2020 10:55:16 +0200 +Subject: vt: keyboard, simplify vt_kdgkbsent + +From: Jiri Slaby + +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 + +--- + 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;