The following changes since commit 4495c08e84729385774601b5146d51d9e5849f81:
Linux 4.11-rc2 (2017-03-12 14:47:08 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.11-rc4
for you to fetch changes up to a4a3e061149f09c075f108b6f1cf04d9739a6bc2:
tty: fix data race in tty_ldisc_ref_wait() (2017-03-17 14:07:10 +0900)
----------------------------------------------------------------
TTY/Serial driver fixes for 4.11-rc4
Here are some tty and serial driver fixes for 4.11-rc4. One of these
fix a long-standing issue in the ldisc code that was found by Dmitry
Vyukov with his great fuzzing work. The other fixes resolve other
reported issues, and there is one revert of a patch in 4.11-rc1 that
wasn't correct.
All of these have been in linux-next for a while with no reported
issues.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
----------------------------------------------------------------
Aleksey Makarov (1):
Revert "tty: serial: pl011: add ttyAMA for matching pl011 console"
Dmitry Vyukov (2):
tty: don't panic on OOM in tty_set_ldisc()
tty: fix data race in tty_ldisc_ref_wait()
Heiko Stuebner (1):
serial: 8250_dw: Honor clk_round_rate errors in dw8250_set_termios
James Hogan (1):
serial: 8250_dw: Fix breakage when HAVE_CLK=n
Timur Tabi (1):
tty: acpi/spcr: QDF2400 E44 checks for wrong OEM revision
drivers/acpi/spcr.c | 2 +-
drivers/tty/serial/8250/8250_dw.c | 9 +++-
drivers/tty/serial/amba-pl011.c | 2 +-
drivers/tty/tty_ldisc.c | 92 +++++++++------------------------------
4 files changed, 30 insertions(+), 75 deletions(-)
On 26 March 2017 at 13:04, Greg KH <[email protected]> wrote:
> The following changes since commit 4495c08e84729385774601b5146d51d9e5849f81:
>
> Linux 4.11-rc2 (2017-03-12 14:47:08 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.11-rc4
>
> for you to fetch changes up to a4a3e061149f09c075f108b6f1cf04d9739a6bc2:
>
> tty: fix data race in tty_ldisc_ref_wait() (2017-03-17 14:07:10 +0900)
>
> ----------------------------------------------------------------
> TTY/Serial driver fixes for 4.11-rc4
>
> Here are some tty and serial driver fixes for 4.11-rc4. One of these
> fix a long-standing issue in the ldisc code that was found by Dmitry
> Vyukov with his great fuzzing work. The other fixes resolve other
> reported issues, and there is one revert of a patch in 4.11-rc1 that
> wasn't correct.
>
> All of these have been in linux-next for a while with no reported
> issues.
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ----------------------------------------------------------------
> Aleksey Makarov (1):
> Revert "tty: serial: pl011: add ttyAMA for matching pl011 console"
>
> Dmitry Vyukov (2):
> tty: don't panic on OOM in tty_set_ldisc()
I've bisected a syzkaller crash down to this commit
(5362544bebe85071188dd9e479b5a5040841c895). The crash is:
[ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
[ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
[ 25.137589] PGD 3b0c067
[ 25.137593] PUD 3911067
[ 25.137597] PMD 0
[ 25.137601]
[ 25.137611] Oops: 0002 [#1] PREEMPT SMP KASAN
[ 25.137624] CPU: 1 PID: 3690 Comm: a.out Not tainted 4.11.0-rc2+ #145
[ 25.137631] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
[ 25.137639] task: ffff880003b96400 task.stack: ffff880004e98000
[ 25.137651] RIP: 0010:mutex_lock_interruptible+0xb/0x30
[ 25.137657] RSP: 0018:ffff880004e9fae0 EFLAGS: 00010246
[ 25.137668] RAX: 0000000000000000 RBX: ffff880004e6c000 RCX: ffffffff817bb2a9
[ 25.137675] RDX: ffff880003b96400 RSI: 0000000000000015 RDI: 0000000000002280
[ 25.137696] RBP: ffff880004e9fca0 R08: 0000000000000003 R09: 0000000000000002
[ 25.137703] R10: 0000000000000002 R11: ffffed0000c23fe9 R12: ffff880004e6c000
[ 25.137710] R13: 0000000080045430 R14: ffff880004bac900 R15: ffff880004bacb60
[ 25.137720] FS: 00007f7cac233700(0000) GS:ffff880006100000(0000)
knlGS:0000000000000000
[ 25.137727] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 25.137733] CR2: 0000000000002280 CR3: 0000000003b67000 CR4: 00000000000006e0
[ 25.137746] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 25.137752] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 25.137755] Call Trace:
[ 25.137769] ? n_tty_read+0x15f/0xc70
[ 25.137783] ? preempt_count_add+0xb2/0xe0
[ 25.137793] ? n_tty_flush_buffer+0x90/0x90
[ 25.137806] ? wait_woken+0x100/0x100
[ 25.137817] tty_read+0xd8/0x140
[ 25.137830] __vfs_read+0xd1/0x320
[ 25.137842] ? do_sendfile+0x6c0/0x6c0
[ 25.137853] ? __fsnotify_update_child_dentry_flags+0x30/0x30
[ 25.137864] ? selinux_file_permission+0x1c0/0x210
[ 25.137873] ? __fsnotify_parent+0x27/0x130
[ 25.137882] ? security_file_permission+0xce/0xf0
[ 25.137893] ? rw_verify_area+0x73/0x140
[ 25.137904] vfs_read+0xba/0x1b0
[ 25.137915] SyS_read+0xa0/0x120
[ 25.137926] ? vfs_write+0x260/0x260
[ 25.137938] ? preempt_count_sub+0x13/0xd0
[ 25.137949] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 25.137957] RIP: 0033:0x7f7caf61351d
[ 25.137963] RSP: 002b:00007f7cac232f20 EFLAGS: 00000293 ORIG_RAX:
0000000000000000
[ 25.137974] RAX: ffffffffffffffda RBX: 00007f7cac233700 RCX: 00007f7caf61351d
[ 25.137980] RDX: 000000000000003e RSI: 0000000080045430 RDI: 0000000000000004
[ 25.137987] RBP: 00007fffb4f21250 R08: 00007f7cac233700 R09: 00007f7cac233700
[ 25.137993] R10: 00007f7cac2339d0 R11: 0000000000000293 R12: 0000000000000000
[ 25.137999] R13: 00007fffb4f2124f R14: 00007f7cac2339c0 R15: 0000000000000000
[ 25.138002] Code: c7 43 20 00 00 00 00 48 89 df e8 91 ff ff ff 5b
41 5c 5d c3 83 e8 01 41 89 44 24 10 eb e1 66 90 65 48 8b 14 25 40 54
01 00 31 c0 <f0> 48 0f b1 17 48 85 c0 74 0a 55 48 89 e5 e8 e2 f4 ff ff
5d f3
[ 25.138218] RIP: mutex_lock_interruptible+0xb/0x30 RSP: ffff880004e9fae0
[ 25.138221] CR2: 0000000000002280
[ 25.138301] ---[ end trace 242fd54c56b177b4 ]---
The syzkaller reproducer is:
# {Threaded:true Collide:true Repeat:true Procs:1 Sandbox:setuid Repro:false}
mmap(&(0x7f0000000000/0x9f000)=nil, (0x9f000), 0x3, 0x32,
0xffffffffffffffff, 0x0)
r0 = openat$ptmx(0xffffffffffffff9c,
&(0x7f0000001000-0xa)="2f6465762f70746d7800", 0x201, 0x0)
ioctl$TIOCSPTLCK(r0, 0x40045431, &(0x7f000009a000)=0x0)
r1 = syz_open_pts(r0, 0x0)
read(r1, &(0x7f0000028000-0x86)="0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
0x3e)
ioctl$TIOCSETD(r1, 0x5423, &(0x7f000009f000-0x4)=0x10080000001)
It takes 10-50 seconds to reproduce, but you can reduce it to ~2
seconds by opening /dev/ptmx only once in each process.
I've verified that reverting the commit from latest mainline makes the
crash go away.
Vegard
On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <[email protected]> wrote:
>
> I've bisected a syzkaller crash down to this commit
> (5362544bebe85071188dd9e479b5a5040841c895). The crash is:
>
> [ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
> [ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
It would seem to be the
if (mutex_lock_interruptible(&ldata->atomic_read_lock))
call in n_tty_read(), the offset is about right for a NULL 'ldata'
pointer (it's a big structure, it has a couple of character buffers of
size N_TTY_BUF_SIZE).
I don't see the obvious fix, so I suspect at this point we should just
revert, as that commit seems to introduce worse problems that it is
supposed to fix. Greg?
Linus
On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <[email protected]> wrote:
> >
> > I've bisected a syzkaller crash down to this commit
> > (5362544bebe85071188dd9e479b5a5040841c895). The crash is:
> >
> > [ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
> > [ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
>
> It would seem to be the
>
> if (mutex_lock_interruptible(&ldata->atomic_read_lock))
>
> call in n_tty_read(), the offset is about right for a NULL 'ldata'
> pointer (it's a big structure, it has a couple of character buffers of
> size N_TTY_BUF_SIZE).
>
> I don't see the obvious fix, so I suspect at this point we should just
> revert, as that commit seems to introduce worse problems that it is
> supposed to fix. Greg?
Unless Dmitry has a better idea, I will just revert it and send you the
pull request in a day or so.
thanks,
greg k-h
On 13 April 2017 at 20:34, Greg KH <[email protected]> wrote:
> On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
>> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <[email protected]> wrote:
>> >
>> > I've bisected a syzkaller crash down to this commit
>> > (5362544bebe85071188dd9e479b5a5040841c895). The crash is:
>> >
>> > [ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
>> > [ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
>>
>> It would seem to be the
>>
>> if (mutex_lock_interruptible(&ldata->atomic_read_lock))
>>
>> call in n_tty_read(), the offset is about right for a NULL 'ldata'
>> pointer (it's a big structure, it has a couple of character buffers of
>> size N_TTY_BUF_SIZE).
>>
>> I don't see the obvious fix, so I suspect at this point we should just
>> revert, as that commit seems to introduce worse problems that it is
>> supposed to fix. Greg?
>
> Unless Dmitry has a better idea, I will just revert it and send you the
> pull request in a day or so.
I don't think we need to rush a revert, I'd hope there's a way to fix
it properly.
So the original problem is that the vmalloc() in n_tty_open() can
fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
because of its unwillingness to proceed if the tty doesn't have an
ldisc.
Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
allocation failure as we can see from the comment in tty_set_ldisc().
Unfortunately, it would appear that some other bits of code do not
like tty->ldisc == NULL (other than the crash in this thread, I saw
2-3 similar crashes in other functions, e.g. poll()). I see two
possibilities:
1) make other code handle tty->ldisc == NULL.
2) don't close/free the old ldisc until the new one has been
successfully created/initialised/opened/attached to the tty, and
return an error to userspace if changing it failed.
I'm leaning towards #2 as the more obviously correct fix, it makes
tty_set_ldisc() transactional, the fix seems limited in scope to
tty_set_ldisc() itself, and we don't need to make every other bit of
code that uses tty->ldisc handle the NULL case.
Vegard
On Fri, Apr 14, 2017 at 11:41:26AM +0200, Vegard Nossum wrote:
> On 13 April 2017 at 20:34, Greg KH <[email protected]> wrote:
> > On Thu, Apr 13, 2017 at 09:07:40AM -0700, Linus Torvalds wrote:
> >> On Thu, Apr 13, 2017 at 3:50 AM, Vegard Nossum <[email protected]> wrote:
> >> >
> >> > I've bisected a syzkaller crash down to this commit
> >> > (5362544bebe85071188dd9e479b5a5040841c895). The crash is:
> >> >
> >> > [ 25.137552] BUG: unable to handle kernel paging request at 0000000000002280
> >> > [ 25.137579] IP: mutex_lock_interruptible+0xb/0x30
> >>
> >> It would seem to be the
> >>
> >> if (mutex_lock_interruptible(&ldata->atomic_read_lock))
> >>
> >> call in n_tty_read(), the offset is about right for a NULL 'ldata'
> >> pointer (it's a big structure, it has a couple of character buffers of
> >> size N_TTY_BUF_SIZE).
> >>
> >> I don't see the obvious fix, so I suspect at this point we should just
> >> revert, as that commit seems to introduce worse problems that it is
> >> supposed to fix. Greg?
> >
> > Unless Dmitry has a better idea, I will just revert it and send you the
> > pull request in a day or so.
>
> I don't think we need to rush a revert, I'd hope there's a way to fix
> it properly.
For this late in the release cycle, for something as complex as tty
ldisc handling, for an issue that has been present for over a decade,
the safest thing right now is to go back to the old well-known code by
applying a revert :)
> So the original problem is that the vmalloc() in n_tty_open() can
> fail, and that will panic in tty_set_ldisc()/tty_ldisc_restore()
> because of its unwillingness to proceed if the tty doesn't have an
> ldisc.
>
> Dmitry fixed this by allowing tty->ldisc == NULL in the case of memory
> allocation failure as we can see from the comment in tty_set_ldisc().
>
> Unfortunately, it would appear that some other bits of code do not
> like tty->ldisc == NULL (other than the crash in this thread, I saw
> 2-3 similar crashes in other functions, e.g. poll()). I see two
> possibilities:
>
> 1) make other code handle tty->ldisc == NULL.
>
> 2) don't close/free the old ldisc until the new one has been
> successfully created/initialised/opened/attached to the tty, and
> return an error to userspace if changing it failed.
>
> I'm leaning towards #2 as the more obviously correct fix, it makes
> tty_set_ldisc() transactional, the fix seems limited in scope to
> tty_set_ldisc() itself, and we don't need to make every other bit of
> code that uses tty->ldisc handle the NULL case.
That sounds reasonable to me, care to work on a patch for this?
thanks,
greg k-h
On Wed, May 31, 2017 at 5:04 PM, Alan Cox <[email protected]> wrote:
> On Wed, 31 May 2017 20:16:12 +0900
> Greg KH <[email protected]> wrote:
>
>> On Wed, May 31, 2017 at 10:39:23AM +0200, Dmitry Vyukov wrote:
>> > On Tue, May 30, 2017 at 2:09 PM, Alan Cox <[email protected]> wrote:
>> > >> >> I'll think about possible solutions, but I have no prior experience
>> > >> >> with the tty code. In the meantime syzkaller also hit a couple of
>> > >> >> other fun tty/pty bugs including a write/ioctl race that results in
>> > >> >> buffer overflow :-/
>> > >
>> > > There are several of those, including some of that have been documented
>> > > for years but nobody ever volunteered to fix - in particular all the
>> > > interfaces that push characters to the tty other than via the normal
>> > > interrupt receive path are dodgy (console selection in particular)
>> > >
>> > > The original tty model btw was that setting the ldisc to n_tty cannot
>> > > fail, and the structure allocated was smaller than a page size so was
>> > > safe.
>> > >
>> > > The simple way to fix it is to restore that behaviour by adding a 'null'
>> > > ldisc that we can fail to instead of N_TTY since the N_TTY failback path
>> > > is long broken.
>> >
>> > Greg, what do you think about this patch? Are you ready to accept
>> > something like this?
>> > Definitely shorter than changing all drivers.
>>
>> Yes, it looks reasonable to me.
>
>
>
> Ok try this
I've applied the patch and run syzkaller with it. I don't see kernel
panics in tty_ldisc_restore any more. Also don't see any new
tty-related crashes.
Greg, will you take it from here?
> commit f6db8de7eca11cfeafa92f2ec866fa75425c5f38
> Author: Alan Cox <[email protected]>
> Date: Tue May 30 12:59:45 2017 +0100
>
> tty: handle the case where we cannot restore a line discipline
>
> Historically the N_TTY driver could never fail but this has become broken over
> time. Rather than trying to rewrite half the ldisc layer to fix the breakage
> introduce a second level of fallback with an N_NULL ldisc which cannot fail,
> and thus restore the guarantees required by the ldisc layer.
>
> We still try and fail to N_TTY first. It's much more useful to find yourself
> back in your old ldisc (first attempt) or in N_TTY (second attempt), and while
> I'm not aware of any code out there that makes those assumptions it's good to
> drive(r) defensively.
>
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index f02becd..8689279 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_TTY) += tty_io.o n_tty.o tty_ioctl.o tty_ldisc.o \
> tty_buffer.o tty_port.o tty_mutex.o \
> - tty_ldsem.o tty_baudrate.o tty_jobctrl.o
> + tty_ldsem.o tty_baudrate.o tty_jobctrl.o \
> + n_null.o
> obj-$(CONFIG_LEGACY_PTYS) += pty.o
> obj-$(CONFIG_UNIX98_PTYS) += pty.o
> obj-$(CONFIG_AUDIT) += tty_audit.o
> diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
> new file mode 100644
> index 0000000..d63261c
> --- /dev/null
> +++ b/drivers/tty/n_null.c
> @@ -0,0 +1,80 @@
> +#include <linux/types.h>
> +#include <linux/errno.h>
> +#include <linux/tty.h>
> +#include <linux/module.h>
> +
> +/*
> + * n_null.c - Null line discipline used in the failure path
> + *
> + * Copyright (C) Intel 2017
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +static int n_null_open(struct tty_struct *tty)
> +{
> + return 0;
> +}
> +
> +static void n_null_close(struct tty_struct *tty)
> +{
> +}
> +
> +static ssize_t n_null_read(struct tty_struct *tty, struct file *file,
> + unsigned char __user * buf, size_t nr)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static ssize_t n_null_write(struct tty_struct *tty, struct file *file,
> + const unsigned char *buf, size_t nr)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static void n_null_receivebuf(struct tty_struct *tty,
> + const unsigned char *cp, char *fp,
> + int cnt)
> +{
> +}
> +
> +static struct tty_ldisc_ops null_ldisc = {
> + .owner = THIS_MODULE,
> + .magic = TTY_LDISC_MAGIC,
> + .name = "n_null",
> + .open = n_null_open,
> + .close = n_null_close,
> + .read = n_null_read,
> + .write = n_null_write,
> + .receive_buf = n_null_receivebuf
> +};
> +
> +static int __init n_null_init(void)
> +{
> + BUG_ON(tty_register_ldisc(N_NULL, &null_ldisc));
> + return 0;
> +}
> +
> +static void __exit n_null_exit(void)
> +{
> + tty_unregister_ldisc(N_NULL);
> +}
> +
> +module_init(n_null_init);
> +module_exit(n_null_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alan Cox");
> +MODULE_ALIAS_LDISC(N_NULL);
> +MODULE_DESCRIPTION("Null ldisc driver");
> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index f6ffe28..2fe216b 100644
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -492,6 +492,29 @@ static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld)
> }
>
> /**
> + * tty_ldisc_failto - helper for ldisc failback
> + * @tty: tty to open the ldisc on
> + * @ld: ldisc we are trying to fail back to
> + *
> + * Helper to try and recover a tty when switching back to the old
> + * ldisc fails and we need something attached.
> + */
> +
> +static int tty_ldisc_failto(struct tty_struct *tty, int ld)
> +{
> + struct tty_ldisc *disc = tty_ldisc_get(tty, ld);
> + int r;
> +
> + if (IS_ERR(disc))
> + return PTR_ERR(disc);
> + tty->ldisc = disc;
> + tty_set_termios_ldisc(tty, ld);
> + if ((r = tty_ldisc_open(tty, disc)) < 0)
> + tty_ldisc_put(disc);
> + return r;
> +}
> +
> +/**
> * tty_ldisc_restore - helper for tty ldisc change
> * @tty: tty to recover
> * @old: previous ldisc
> @@ -502,9 +525,6 @@ static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld)
>
> static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
> {
> - struct tty_ldisc *new_ldisc;
> - int r;
> -
> /* There is an outstanding reference here so this is safe */
> old = tty_ldisc_get(tty, old->ops->num);
> WARN_ON(IS_ERR(old));
> @@ -512,17 +532,13 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
> tty_set_termios_ldisc(tty, old->ops->num);
> if (tty_ldisc_open(tty, old) < 0) {
> tty_ldisc_put(old);
> - /* This driver is always present */
> - new_ldisc = tty_ldisc_get(tty, N_TTY);
> - if (IS_ERR(new_ldisc))
> - panic("n_tty: get");
> - tty->ldisc = new_ldisc;
> - tty_set_termios_ldisc(tty, N_TTY);
> - r = tty_ldisc_open(tty, new_ldisc);
> - if (r < 0)
> - panic("Couldn't open N_TTY ldisc for "
> - "%s --- error %d.",
> - tty_name(tty), r);
> + /* The traditional behaviour is to fall back to N_TTY, we
> + want to avoid falling back to N_NULL unless we have no
> + choice to avoid the risk of breaking anything */
> + if (tty_ldisc_failto(tty, N_TTY) < 0 &&
> + tty_ldisc_failto(tty, N_NULL) < 0)
> + panic("Couldn't open N_NULL ldisc for %s.",
> + tty_name(tty));
> }
> }
>
> diff --git a/include/uapi/linux/tty.h b/include/uapi/linux/tty.h
> index e7855df..cf14553 100644
> --- a/include/uapi/linux/tty.h
> +++ b/include/uapi/linux/tty.h
> @@ -36,5 +36,6 @@
> #define N_TRACEROUTER 24 /* Trace data routing for MIPI P1149.7 */
> #define N_NCI 25 /* NFC NCI UART */
> #define N_SPEAKUP 26 /* Speakup communication with synths */
> +#define N_NULL 27 /* Null ldisc used for error handling */
>
> #endif /* _UAPI_LINUX_TTY_H */
On Thu, Jun 01, 2017 at 02:06:08PM +0200, Dmitry Vyukov wrote:
> On Wed, May 31, 2017 at 5:04 PM, Alan Cox <[email protected]> wrote:
> > On Wed, 31 May 2017 20:16:12 +0900
> > Greg KH <[email protected]> wrote:
> >
> >> On Wed, May 31, 2017 at 10:39:23AM +0200, Dmitry Vyukov wrote:
> >> > On Tue, May 30, 2017 at 2:09 PM, Alan Cox <[email protected]> wrote:
> >> > >> >> I'll think about possible solutions, but I have no prior experience
> >> > >> >> with the tty code. In the meantime syzkaller also hit a couple of
> >> > >> >> other fun tty/pty bugs including a write/ioctl race that results in
> >> > >> >> buffer overflow :-/
> >> > >
> >> > > There are several of those, including some of that have been documented
> >> > > for years but nobody ever volunteered to fix - in particular all the
> >> > > interfaces that push characters to the tty other than via the normal
> >> > > interrupt receive path are dodgy (console selection in particular)
> >> > >
> >> > > The original tty model btw was that setting the ldisc to n_tty cannot
> >> > > fail, and the structure allocated was smaller than a page size so was
> >> > > safe.
> >> > >
> >> > > The simple way to fix it is to restore that behaviour by adding a 'null'
> >> > > ldisc that we can fail to instead of N_TTY since the N_TTY failback path
> >> > > is long broken.
> >> >
> >> > Greg, what do you think about this patch? Are you ready to accept
> >> > something like this?
> >> > Definitely shorter than changing all drivers.
> >>
> >> Yes, it looks reasonable to me.
> >
> >
> >
> > Ok try this
>
>
> I've applied the patch and run syzkaller with it. I don't see kernel
> panics in tty_ldisc_restore any more. Also don't see any new
> tty-related crashes.
>
> Greg, will you take it from here?
I can if Alan sends it to me in a form I can apply it in (i.e. it has a
siged-off-by line...)
thanks,
greg k-h