* [PATCH 1/2] vt: selection, handle pending signals in paste_selection @ 2020-02-10 8:11 Jiri Slaby 2020-02-10 8:11 ` [PATCH 2/2] vt: selection, close sel_buffer race Jiri Slaby 0 siblings, 1 reply; 5+ messages in thread From: Jiri Slaby @ 2020-02-10 8:11 UTC (permalink / raw) To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby 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> --- 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 78732feaf65b..44d974d4159f 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -29,6 +29,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) == ' ') @@ -350,6 +352,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(); @@ -363,6 +366,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; @@ -378,6 +385,6 @@ int paste_selection(struct tty_struct *tty) tty_buffer_unlock_exclusive(&vc->port); tty_ldisc_deref(ld); - return 0; + return ret; } EXPORT_SYMBOL_GPL(paste_selection); -- 2.25.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] vt: selection, close sel_buffer race 2020-02-10 8:11 [PATCH 1/2] vt: selection, handle pending signals in paste_selection Jiri Slaby @ 2020-02-10 8:11 ` Jiri Slaby 2020-02-12 19:59 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: Jiri Slaby @ 2020-02-10 8:11 UTC (permalink / raw) To: gregkh Cc: linux-serial, linux-kernel, Jiri Slaby, syzbot+59997e8d5cbdc486e6f6 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 --- 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 44d974d4159f..0c50d7410b31 100644 --- a/drivers/tty/vt/selection.c +++ b/drivers/tty/vt/selection.c @@ -16,6 +16,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> @@ -45,6 +46,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) */ @@ -186,7 +188,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) char *bp, *obp; int i, ps, pe, multiplier; u32 c; - int mode; + int mode, ret = 0; poke_blanked_console(); @@ -212,6 +214,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) 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; @@ -257,9 +260,10 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) break; case TIOCL_SELPOINTER: highlight_pointer(pe); - return 0; + goto unlock; default: - return -EINVAL; + ret = -EINVAL; + goto unlock; } /* remove the pointer */ @@ -281,7 +285,7 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) 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 */ @@ -309,7 +313,8 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) if (!bp) { printk(KERN_WARNING "selection: kmalloc() failed\n"); clear_selection(); - return -ENOMEM; + ret = -ENOMEM; + goto unlock; } kfree(sel_buffer); sel_buffer = bp; @@ -334,7 +339,9 @@ int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty) } } sel_buffer_lth = bp - sel_buffer; - return 0; +unlock: + mutex_unlock(&sel_lock); + return ret; } EXPORT_SYMBOL_GPL(set_selection_kernel); @@ -364,6 +371,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)) { @@ -371,7 +379,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); @@ -380,6 +390,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); -- 2.25.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] vt: selection, close sel_buffer race 2020-02-10 8:11 ` [PATCH 2/2] vt: selection, close sel_buffer race Jiri Slaby @ 2020-02-12 19:59 ` Greg KH 2020-02-13 7:09 ` Jiri Slaby 0 siblings, 1 reply; 5+ messages in thread From: Greg KH @ 2020-02-12 19:59 UTC (permalink / raw) To: Jiri Slaby; +Cc: linux-serial, linux-kernel, syzbot+59997e8d5cbdc486e6f6 On Mon, Feb 10, 2020 at 09:11:31AM +0100, Jiri Slaby wrote: > 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 This needs patch 1 in order to work properly, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] vt: selection, close sel_buffer race 2020-02-12 19:59 ` Greg KH @ 2020-02-13 7:09 ` Jiri Slaby 2020-02-13 19:58 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: Jiri Slaby @ 2020-02-13 7:09 UTC (permalink / raw) To: Greg KH; +Cc: linux-serial, linux-kernel, syzbot+59997e8d5cbdc486e6f6 On 12. 02. 20, 20:59, Greg KH wrote: > On Mon, Feb 10, 2020 at 09:11:31AM +0100, Jiri Slaby wrote: >> 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 > > This needs patch 1 in order to work properly, right? Not necessarily -- the patches fix two different bugs (endless loop in kernel vs crash). If you want to apply them in the opposite order, just let me know. BTW I completely forgot to add Fixes and Cc: stable tags. Both of the issues come from 1.x times. (But the crash obviously needs SMP/preemption, i.e. 2.x.*.) thanks, -- js suse labs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] vt: selection, close sel_buffer race 2020-02-13 7:09 ` Jiri Slaby @ 2020-02-13 19:58 ` Greg KH 0 siblings, 0 replies; 5+ messages in thread From: Greg KH @ 2020-02-13 19:58 UTC (permalink / raw) To: Jiri Slaby; +Cc: linux-serial, linux-kernel, syzbot+59997e8d5cbdc486e6f6 On Thu, Feb 13, 2020 at 08:09:31AM +0100, Jiri Slaby wrote: > On 12. 02. 20, 20:59, Greg KH wrote: > > On Mon, Feb 10, 2020 at 09:11:31AM +0100, Jiri Slaby wrote: > >> 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 > > > > This needs patch 1 in order to work properly, right? > > Not necessarily -- the patches fix two different bugs (endless loop in > kernel vs crash). If you want to apply them in the opposite order, just > let me know. > > BTW I completely forgot to add Fixes and Cc: stable tags. Both of the > issues come from 1.x times. (But the crash obviously needs > SMP/preemption, i.e. 2.x.*.) No worries, I'll go add cc: stable to them now, thanks. greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-13 19:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-10 8:11 [PATCH 1/2] vt: selection, handle pending signals in paste_selection Jiri Slaby 2020-02-10 8:11 ` [PATCH 2/2] vt: selection, close sel_buffer race Jiri Slaby 2020-02-12 19:59 ` Greg KH 2020-02-13 7:09 ` Jiri Slaby 2020-02-13 19:58 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).