Commit 215c1df2 authored by Jiri Slaby's avatar Jiri Slaby Committed by Kelsey Skunberg

vt: selection, close sel_buffer race

BugLink: https://bugs.launchpad.net/bugs/1868628

commit 07e6124a upstream.

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: default avatarJiri 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.czSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
Signed-off-by: default avatarKelsey Skunberg <kelsey.skunberg@canonical.com>
parent f4c07593
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <linux/tty.h> #include <linux/tty.h>
#include <linux/sched.h> #include <linux/sched.h>
#include <linux/mm.h> #include <linux/mm.h>
#include <linux/mutex.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/types.h> #include <linux/types.h>
...@@ -40,6 +41,7 @@ static volatile int sel_start = -1; /* cleared by clear_selection */ ...@@ -40,6 +41,7 @@ static volatile int sel_start = -1; /* cleared by clear_selection */
static int sel_end; static int sel_end;
static int sel_buffer_lth; static int sel_buffer_lth;
static char *sel_buffer; static char *sel_buffer;
static DEFINE_MUTEX(sel_lock);
/* clear_selection, highlight and highlight_pointer can be called /* clear_selection, highlight and highlight_pointer can be called
from interrupt (via scrollback/front) */ from interrupt (via scrollback/front) */
...@@ -163,7 +165,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -163,7 +165,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
char *bp, *obp; char *bp, *obp;
int i, ps, pe, multiplier; int i, ps, pe, multiplier;
u16 c; u16 c;
int mode; int mode, ret = 0;
poke_blanked_console(); poke_blanked_console();
...@@ -203,6 +205,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -203,6 +205,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
pe = tmp; pe = tmp;
} }
mutex_lock(&sel_lock);
if (sel_cons != vc_cons[fg_console].d) { if (sel_cons != vc_cons[fg_console].d) {
clear_selection(); clear_selection();
sel_cons = vc_cons[fg_console].d; sel_cons = vc_cons[fg_console].d;
...@@ -248,9 +251,10 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -248,9 +251,10 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
break; break;
case TIOCL_SELPOINTER: case TIOCL_SELPOINTER:
highlight_pointer(pe); highlight_pointer(pe);
return 0; goto unlock;
default: default:
return -EINVAL; ret = -EINVAL;
goto unlock;
} }
/* remove the pointer */ /* remove the pointer */
...@@ -272,7 +276,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -272,7 +276,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
else if (new_sel_start == sel_start) else if (new_sel_start == sel_start)
{ {
if (new_sel_end == sel_end) /* no action required */ if (new_sel_end == sel_end) /* no action required */
return 0; goto unlock;
else if (new_sel_end > sel_end) /* extend to right */ else if (new_sel_end > sel_end) /* extend to right */
highlight(sel_end + 2, new_sel_end); highlight(sel_end + 2, new_sel_end);
else /* contract from right */ else /* contract from right */
...@@ -299,7 +303,8 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -299,7 +303,8 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
if (!bp) { if (!bp) {
printk(KERN_WARNING "selection: kmalloc() failed\n"); printk(KERN_WARNING "selection: kmalloc() failed\n");
clear_selection(); clear_selection();
return -ENOMEM; ret = -ENOMEM;
goto unlock;
} }
kfree(sel_buffer); kfree(sel_buffer);
sel_buffer = bp; sel_buffer = bp;
...@@ -324,7 +329,9 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t ...@@ -324,7 +329,9 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
} }
} }
sel_buffer_lth = bp - sel_buffer; sel_buffer_lth = bp - sel_buffer;
return 0; unlock:
mutex_unlock(&sel_lock);
return ret;
} }
/* Insert the contents of the selection buffer into the /* Insert the contents of the selection buffer into the
...@@ -352,6 +359,7 @@ int paste_selection(struct tty_struct *tty) ...@@ -352,6 +359,7 @@ int paste_selection(struct tty_struct *tty)
tty_buffer_lock_exclusive(&vc->port); tty_buffer_lock_exclusive(&vc->port);
add_wait_queue(&vc->paste_wait, &wait); add_wait_queue(&vc->paste_wait, &wait);
mutex_lock(&sel_lock);
while (sel_buffer && sel_buffer_lth > pasted) { while (sel_buffer && sel_buffer_lth > pasted) {
set_current_state(TASK_INTERRUPTIBLE); set_current_state(TASK_INTERRUPTIBLE);
if (test_bit(TTY_THROTTLED, &tty->flags)) { if (test_bit(TTY_THROTTLED, &tty->flags)) {
...@@ -364,6 +372,7 @@ int paste_selection(struct tty_struct *tty) ...@@ -364,6 +372,7 @@ int paste_selection(struct tty_struct *tty)
count); count);
pasted += count; pasted += count;
} }
mutex_unlock(&sel_lock);
remove_wait_queue(&vc->paste_wait, &wait); remove_wait_queue(&vc->paste_wait, &wait);
__set_current_state(TASK_RUNNING); __set_current_state(TASK_RUNNING);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment