Skip to content

Commit

Permalink
vt: selection, close sel_buffer race
Browse files Browse the repository at this point in the history
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.

Bug 2977923

Change-Id: I77ab910c50c3cea6f99d5fbd947dacf3342d2578
Signed-off-by: Jiri Slaby <[email protected]>
Reported-by: [email protected]
References: https://bugzilla.kernel.org/show_bug.cgi?id=206361
Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Reviewed-on: https://git-master.nvidia.com/r/c/linux-4.9/+/2407650
Reviewed-by: automaticguardword <[email protected]>
Reviewed-by: Bibek Basu <[email protected]>
Reviewed-by: mobile promotions <[email protected]>
GVS: Gerrit_Virtual_Submit
Tested-by: Amulya Yarlagadda <[email protected]>
Tested-by: mobile promotions <[email protected]>
  • Loading branch information
amulya Y authored and mobile promotions committed Sep 9, 2020
1 parent c1a6ff0 commit 470ac70
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions drivers/tty/vt/selection.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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>

Expand Down Expand Up @@ -40,6 +41,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) */
Expand Down Expand Up @@ -163,7 +165,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();

Expand Down Expand Up @@ -203,6 +205,7 @@ int set_selection(const struct tiocl_selection __user *sel, struct tty_struct *t
pe = tmp;
}

mutex_lock(&sel_lock);
if (sel_cons != vc_cons[fg_console].d) {
clear_selection();
sel_cons = vc_cons[fg_console].d;
Expand Down Expand Up @@ -248,9 +251,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 */
Expand All @@ -272,7 +276,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 */
Expand All @@ -299,7 +303,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;
Expand All @@ -324,7 +329,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
Expand Down Expand Up @@ -352,10 +359,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);
Expand All @@ -364,6 +374,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);

Expand Down

0 comments on commit 470ac70

Please sign in to comment.