Jiri Slaby (2): vt: selection, handle pending signals in paste_selection vt: selection, close sel_buffer race
drivers/tty/vt/selection.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)
From: Jiri Slaby jslaby@suse.cz
mainline inclusion from mainline-v5.6-rc2 commit 687bff0cd08f790d540cfb7b2349f0d876cdddec category: bugfix bugzilla: 13690 CVE: CVE-2020-8648
-------------------------------------------------
When pasting a selection to a vt, the task is set as INTERRUPTIBLE while waiting for a tty to unthrottle. But signals are not handled at all. Normally, this is not a problem as tty_ldisc_receive_buf receives all the goods and a user has no reason to interrupt the task.
There are two scenarios where this matters: 1) when the tty is throttled and a signal is sent to the process, it spins on a CPU until the tty is unthrottled. schedule() does not really echedule, but returns immediately, of course. 2) when the sel_buffer becomes invalid, KASAN prevents any reads from it and the loop simply does not proceed and spins forever (causing the tty to throttle, but the code never sleeps, the same as above). This sometimes happens as there is a race in the sel_buffer handling code.
So add signal handling to this ioctl (TIOCL_PASTESEL) and return -EINTR in case a signal is pending.
Signed-off-by: Jiri Slaby jslaby@suse.cz Cc: stable stable@vger.kernel.org Link: https://lore.kernel.org/r/20200210081131.23572-1-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yang Yingliang yangyingliang@huawei.com Reviewed-by: Hanjun Guo guohanjun@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- drivers/tty/vt/selection.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 07496c7..3ac4fe5 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -27,6 +27,8 @@ #include <linux/console.h> #include <linux/tty_flip.h>
+#include <linux/sched/signal.h> + /* Don't take this from <ctype.h>: 011-015 on the screen aren't spaces */ #define isspace(c) ((c) == ' ')
@@ -337,6 +339,7 @@ int paste_selection(struct tty_struct *tty) unsigned int count; struct tty_ldisc *ld; DECLARE_WAITQUEUE(wait, current); + int ret = 0;
console_lock(); poke_blanked_console(); @@ -350,6 +353,10 @@ int paste_selection(struct tty_struct *tty) add_wait_queue(&vc->paste_wait, &wait); while (sel_buffer && sel_buffer_lth > pasted) { set_current_state(TASK_INTERRUPTIBLE); + if (signal_pending(current)) { + ret = -EINTR; + break; + } if (tty_throttled(tty)) { schedule(); continue; @@ -365,5 +372,5 @@ int paste_selection(struct tty_struct *tty)
tty_buffer_unlock_exclusive(&vc->port); tty_ldisc_deref(ld); - return 0; + return ret; }
From: Jiri Slaby jslaby@suse.cz
mainline inclusion from mainline-v5.6-rc2 commit 07e6124a1a46b4b5a9b3cacc0c306b50da87abf5 category: bugfix bugzilla: 13690 CVE: CVE-2020-8648
-------------------------------------------------
syzkaller reported this UAF: BUG: KASAN: use-after-free in n_tty_receive_buf_common+0x2481/0x2940 drivers/tty/n_tty.c:1741 Read of size 1 at addr ffff8880089e40e9 by task syz-executor.1/13184
CPU: 0 PID: 13184 Comm: syz-executor.1 Not tainted 5.4.7 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 Call Trace: ... kasan_report+0xe/0x20 mm/kasan/common.c:634 n_tty_receive_buf_common+0x2481/0x2940 drivers/tty/n_tty.c:1741 tty_ldisc_receive_buf+0xac/0x190 drivers/tty/tty_buffer.c:461 paste_selection+0x297/0x400 drivers/tty/vt/selection.c:372 tioclinux+0x20d/0x4e0 drivers/tty/vt/vt.c:3044 vt_ioctl+0x1bcf/0x28d0 drivers/tty/vt/vt_ioctl.c:364 tty_ioctl+0x525/0x15a0 drivers/tty/tty_io.c:2657 vfs_ioctl fs/ioctl.c:47 [inline]
It is due to a race between parallel paste_selection (TIOCL_PASTESEL) and set_selection_user (TIOCL_SETSEL) invocations. One uses sel_buffer, while the other frees it and reallocates a new one for another selection. Add a mutex to close this race.
The mutex takes care properly of sel_buffer and sel_buffer_lth only. The other selection global variables (like sel_start, sel_end, and sel_cons) are protected only in set_selection_user. The other functions need quite some more work to close the races of the variables there. This is going to happen later.
This likely fixes (I am unsure as there is no reproducer provided) bug 206361 too. It was marked as CVE-2020-8648.
Signed-off-by: Jiri Slaby jslaby@suse.cz Reported-by: syzbot+59997e8d5cbdc486e6f6@syzkaller.appspotmail.com References: https://bugzilla.kernel.org/show_bug.cgi?id=206361 Cc: stable stable@vger.kernel.org Link: https://lore.kernel.org/r/20200210081131.23572-2-jslaby@suse.cz Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Yang Yingliang yangyingliang@huawei.com Reviewed-by: Hanjun Guo guohanjun@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- drivers/tty/vt/selection.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c index 3ac4fe5..69102fb 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -14,6 +14,7 @@ #include <linux/tty.h> #include <linux/sched.h> #include <linux/mm.h> +#include <linux/mutex.h> #include <linux/slab.h> #include <linux/types.h>
@@ -43,6 +44,7 @@ static int sel_end; static int sel_buffer_lth; static char *sel_buffer; +static DEFINE_MUTEX(sel_lock);
/* clear_selection, highlight and highlight_pointer can be called from interrupt (via scrollback/front) */ @@ -173,7 +175,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t char *bp, *obp; int i, ps, pe, multiplier; u32 c; - int mode; + int mode, ret = 0;
poke_blanked_console(); if (copy_from_user(&v, sel, sizeof(*sel))) @@ -200,6 +202,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t if (ps > pe) /* make sel_start <= sel_end */ swap(ps, pe);
+ mutex_lock(&sel_lock); if (sel_cons != vc_cons[fg_console].d) { clear_selection(); sel_cons = vc_cons[fg_console].d; @@ -245,9 +248,10 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t break; case TIOCL_SELPOINTER: highlight_pointer(pe); - return 0; + goto unlock; default: - return -EINVAL; + ret = -EINVAL; + goto unlock; }
/* remove the pointer */ @@ -269,7 +273,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t else if (new_sel_start == sel_start) { if (new_sel_end == sel_end) /* no action required */ - return 0; + goto unlock; else if (new_sel_end > sel_end) /* extend to right */ highlight(sel_end + 2, new_sel_end); else /* contract from right */ @@ -297,7 +301,8 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t if (!bp) { printk(KERN_WARNING "selection: kmalloc() failed\n"); clear_selection(); - return -ENOMEM; + ret = -ENOMEM; + goto unlock; } kfree(sel_buffer); sel_buffer = bp; @@ -322,7 +327,9 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t } } sel_buffer_lth = bp - sel_buffer; - return 0; +unlock: + mutex_unlock(&sel_lock); + return ret; }
/* Insert the contents of the selection buffer into the @@ -351,6 +358,7 @@ int paste_selection(struct tty_struct *tty) tty_buffer_lock_exclusive(&vc->port);
add_wait_queue(&vc->paste_wait, &wait); + mutex_lock(&sel_lock); while (sel_buffer && sel_buffer_lth > pasted) { set_current_state(TASK_INTERRUPTIBLE); if (signal_pending(current)) { @@ -358,7 +366,9 @@ int paste_selection(struct tty_struct *tty) break; } if (tty_throttled(tty)) { + mutex_unlock(&sel_lock); schedule(); + mutex_lock(&sel_lock); continue; } __set_current_state(TASK_RUNNING); @@ -367,6 +377,7 @@ int paste_selection(struct tty_struct *tty) count); pasted += count; } + mutex_unlock(&sel_lock); remove_wait_queue(&vc->paste_wait, &wait); __set_current_state(TASK_RUNNING);
-
Yang Yingliang