[RHEL8,COMMIT] ms/vt: selection, close sel_buffer race #PSBM-120640

Submitted by Konstantin Khorenko on Oct. 14, 2020, 12:32 p.m.

Details

Message ID 202010141232.09ECWq2Z1528360@finist-co8.sw.ru
State New
Series "Fixes for CVE-2020-8648"
Headers show

Commit Message

Konstantin Khorenko Oct. 14, 2020, 12:32 p.m.
The commit is pushed to "branch-rh8-4.18.0-193.6.3.vz8.4.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-193.6.3.vz8.4.12
------>
commit c69dd934d249b86e9b493bd89261b03f70f2357b
Author: Jiri Slaby <jslaby@suse.cz>
Date:   Wed Oct 14 15:32:52 2020 +0300

    ms/vt: selection, close sel_buffer race #PSBM-120640
    
    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>
    
    https://jira.sw.ru/browse/PSBM-120640
    
    This is a backport of mainline commit 07e6124a1a46b4b5a9b3cacc0c306b50da87abf5:
    the affected code is in set_selection() rather than set_selection_kernel().
    
    Signed-off-by: Evgenii Shatokhin <eshatokhin@virtuozzo.com>
---
 drivers/tty/vt/selection.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 90ea1cc52b7a..2a68d6fdb7b1 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>
 
@@ -41,6 +42,7 @@  static volatile int sel_start = -1; 	/* cleared by clear_selection */
 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) */
@@ -161,7 +163,7 @@  int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
 	char *bp, *obp;
 	int i, ps, pe, multiplier;
 	u16 c;
-	int mode;
+	int mode, ret = 0;
 
 	poke_blanked_console();
 	if (copy_from_user(&v, sel, sizeof(*sel)))
@@ -188,6 +190,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;
@@ -233,9 +236,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 */
@@ -257,7 +261,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 */
@@ -285,7 +289,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;
@@ -310,7 +315,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
@@ -338,10 +345,13 @@  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 (tty_throttled(tty)) {
+			mutex_unlock(&sel_lock);
 			schedule();
+			mutex_lock(&sel_lock);
 			continue;
 		}
 		__set_current_state(TASK_RUNNING);
@@ -350,6 +360,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);