From 05c7cd39907184328f48d3e7899f9cdd653ad336 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 3 Jan 2013 15:53:04 +0100 Subject: TTY: switch tty_insert_flip_string Now, we start converting tty buffer functions to actually use tty_port. This will allow us to get rid of the need of tty in many call sites. Only tty_port will needed and hence no more tty_port_tty_get in those paths. tty_insert_flip_string this time. Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index be6a373601b7..3c285d398f38 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -120,7 +120,7 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c) if (c > 0) { /* Stuff the data into the input queue of the other end */ - c = tty_insert_flip_string(to, buf, c); + c = tty_insert_flip_string(to->port, buf, c); /* And shovel */ if (c) { tty_flip_buffer_push(to); -- cgit v1.2.3 From 2e124b4a390ca85325fae75764bef92f0547fa25 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Thu, 3 Jan 2013 15:53:06 +0100 Subject: TTY: switch tty_flip_buffer_push Now, we start converting tty buffer functions to actually use tty_port. This will allow us to get rid of the need of tty in many call sites. Only tty_port will needed and hence no more tty_port_tty_get in those paths. Now, the one where most of tty_port_tty_get gets removed: tty_flip_buffer_push. IOW we also closed all the races in drivers not using tty_port_tty_get at all yet. Also we move tty_flip_buffer_push declaration from include/linux/tty.h to include/linux/tty_flip.h to all others while we are changing it anyway. Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 3c285d398f38..32d027c303aa 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -123,7 +123,7 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c) c = tty_insert_flip_string(to->port, buf, c); /* And shovel */ if (c) { - tty_flip_buffer_push(to); + tty_flip_buffer_push(to->port); tty_wakeup(tty); } } -- cgit v1.2.3 From 82f8c35f86a878a504f92559d631ea03f0f62152 Mon Sep 17 00:00:00 2001 From: Cong Ding Date: Sat, 12 Jan 2013 05:01:21 +0100 Subject: tty: cleanup the panic message the "\n" in panic message is excess, so we remove it in tty/pty.c as what it is used in other places. Signed-off-by: Cong Ding Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 32d027c303aa..916825f984a9 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -795,7 +795,7 @@ static void __init unix98_pty_init(void) cdev_init(&ptmx_cdev, &ptmx_fops); if (cdev_add(&ptmx_cdev, MKDEV(TTYAUX_MAJOR, 2), 1) || register_chrdev_region(MKDEV(TTYAUX_MAJOR, 2), 1, "/dev/ptmx") < 0) - panic("Couldn't register /dev/ptmx driver\n"); + panic("Couldn't register /dev/ptmx driver"); device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 2), NULL, "ptmx"); } -- cgit v1.2.3 From b9f8033f28448732612e64046b13087b08dd25f7 Mon Sep 17 00:00:00 2001 From: Cong Ding Date: Sat, 12 Jan 2013 05:01:22 +0100 Subject: tty: cleanup checkpatch warning in pty.c spaces are used for indent in 3 places of tty/pty.c, we change it to tab. Signed-off-by: Cong Ding Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 916825f984a9..4ec11f326d6d 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -55,9 +55,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp) set_bit(TTY_OTHER_CLOSED, &tty->flags); #ifdef CONFIG_UNIX98_PTYS if (tty->driver == ptm_driver) { - mutex_lock(&devpts_mutex); + mutex_lock(&devpts_mutex); devpts_pty_kill(tty->link->driver_data); - mutex_unlock(&devpts_mutex); + mutex_unlock(&devpts_mutex); } #endif tty_unlock(tty); @@ -661,7 +661,7 @@ static const struct tty_operations pty_unix98_ops = { * Allocate a unix98 pty master device from the ptmx driver. * * Locking: tty_mutex protects the init_dev work. tty->count should - * protect the rest. + * protect the rest. * allocated_ptys_lock handles the list of free pty numbers */ -- cgit v1.2.3 From b81273a132177edd806476b953f6afeb17b786d5 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Tue, 15 Jan 2013 23:26:22 +0100 Subject: TTY: do not reset master's packet mode Now that login from util-linux is forced to drop all references to a TTY which it wants to hangup (to reach reference count 1) we are seeing issues with telnet. When login closes its last reference to the slave PTY, it also resets packet mode on the *master* side. And we have a race here. What telnet does is fork+exec of `login'. Then there are two scenarios: * `login' closes the slave TTY and resets thus master's packet mode, but even now telnet properly sets the mode, or * `telnetd' sets packet mode on the master, `login' closes the slave TTY and resets master's packet mode. The former case is OK. However the latter happens in much more cases, by the order of magnitude to be precise. So when one tries to login to such a messed telnet setup, they see the following: inux login: ogin incorrect Note the missing first letters -- telnet thinks it is still in the packet mode, so when it receives "linux login" from `login', it considers "l" as the type of the packet and strips it. SuS does not mention how the implementation should behave. Both BSDs I checked (Free and Net) do not reset the flag upon the last close. By this I am resurrecting an old bug, see References. We are hitting it regularly now, i.e. with updated util-linux, ergo login. Here, I am changing a behavior introduced back in 2.1 times. It would better have a long time testing before goes upstream. Signed-off-by: Jiri Slaby Cc: Mauro Carvalho Chehab Cc: Bryan Mason References: https://lkml.org/lkml/2009/11/11/223 References: https://bugzilla.redhat.com/show_bug.cgi?id=504703 References: https://bugzilla.novell.com/show_bug.cgi?id=797042 Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 4ec11f326d6d..40ff2bf68b43 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -47,7 +47,6 @@ static void pty_close(struct tty_struct *tty, struct file *filp) /* Review - krefs on tty_link ?? */ if (!tty->link) return; - tty->link->packet = 0; set_bit(TTY_OTHER_CLOSED, &tty->link->flags); wake_up_interruptible(&tty->link->read_wait); wake_up_interruptible(&tty->link->write_wait); -- cgit v1.2.3 From 7acf6cd80b201f77371a5374a786144153629be8 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 30 Jan 2013 12:43:49 -0500 Subject: pty: Fix BUG()s when ptmx_open() errors out If pmtx_open() fails to get a slave inode or fails the pty_open(), the tty is released as part of the error cleanup. As evidenced by the first BUG stacktrace below, pty_close() assumes that the linked pty has a valid, initialized inode* stored in driver_data. Also, as evidenced by the second BUG stacktrace below, pty_unix98_shutdown() assumes that the master pty's driver_data has been initialized. 1) Fix the invalid assumption in pty_close(). 2) Initialize driver_data immediately so proper devpts fs cleanup occurs. Fixes this BUG: [ 815.868844] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028 [ 815.869018] IP: [] devpts_pty_kill+0x1c/0xa0 [ 815.869190] PGD 7c775067 PUD 79deb067 PMD 0 [ 815.869315] Oops: 0000 [#1] PREEMPT SMP [ 815.869443] Modules linked in: kvm_intel kvm snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_seq_midi microcode snd_rawmidi psmouse serio_raw snd_seq_midi_event snd_seq snd_timer$ [ 815.870025] CPU 0 [ 815.870143] Pid: 27819, comm: stress_test_tty Tainted: G W 3.8.0-next-20130125+ttypatch-2-xeon #2 Bochs Bochs [ 815.870386] RIP: 0010:[] [] devpts_pty_kill+0x1c/0xa0 [ 815.870540] RSP: 0018:ffff88007d3e1ac8 EFLAGS: 00010282 [ 815.870661] RAX: ffff880079c20800 RBX: 0000000000000000 RCX: 0000000000000000 [ 815.870804] RDX: ffff880079c209a8 RSI: 0000000000000286 RDI: 0000000000000000 [ 815.870933] RBP: ffff88007d3e1ae8 R08: 0000000000000000 R09: 0000000000000000 [ 815.871078] R10: 0000000000000000 R11: 0000000000000001 R12: ffff88007bfb7e00 [ 815.871209] R13: 0000000000000005 R14: ffff880079c20c00 R15: ffff880079c20c00 [ 815.871343] FS: 00007f2e86206700(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000 [ 815.871495] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 815.871617] CR2: 0000000000000028 CR3: 000000007ae56000 CR4: 00000000000006f0 [ 815.871752] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 815.871902] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 815.872012] Process stress_test_tty (pid: 27819, threadinfo ffff88007d3e0000, task ffff88007c874530) [ 815.872012] Stack: [ 815.872012] ffff88007bfb7e00 ffff880079c20c00 ffff88007bfb7e00 0000000000000005 [ 815.872012] ffff88007d3e1b08 ffffffff81417be7 ffff88007caa9bd8 ffff880079c20800 [ 815.872012] ffff88007d3e1bc8 ffffffff8140e5f8 0000000000000000 0000000000000000 [ 815.872012] Call Trace: [ 815.872012] [] pty_close+0x157/0x170 [ 815.872012] [] tty_release+0x138/0x580 [ 815.872012] [] ? _raw_spin_lock+0x23/0x30 [ 815.872012] [] ? _raw_spin_unlock+0x1a/0x40 [ 815.872012] [] ? __mutex_unlock_slowpath+0x48/0x60 [ 815.872012] [] ptmx_open+0x11f/0x180 [ 815.872012] [] chrdev_open+0x9b/0x1c0 [ 815.872012] [] do_dentry_open+0x203/0x290 [ 815.872012] [] ? cdev_put+0x30/0x30 [ 815.872012] [] finish_open+0x35/0x50 [ 815.872012] [] do_last+0x6fe/0xe90 [ 815.872012] [] ? link_path_walk+0x7f/0x880 [ 815.872012] [] ? cpuacct_charge+0x75/0x80 [ 815.872012] [] path_openat+0xbc/0x4e0 [ 815.872012] [] ? __schedule+0x400/0x7f0 [ 815.872012] [] ? tty_release+0x496/0x580 [ 815.872012] [] do_filp_open+0x41/0xa0 [ 815.872012] [] ? _raw_spin_unlock+0x1a/0x40 [ 815.872012] [] ? __alloc_fd+0xe9/0x140 [ 815.872012] [] do_sys_open+0xf4/0x1e0 [ 815.872012] [] sys_open+0x21/0x30 [ 815.872012] [] system_call_fastpath+0x16/0x1b [ 815.872012] Code: 0f 1f 80 00 00 00 00 45 31 e4 eb d7 0f 0b 90 0f 1f 44 00 00 55 48 89 e5 48 83 ec 20 48 89 5d e8 48 89 fb 4c 89 65 f0 4c 89 6d f8 <48> 8b 47 28 48 81 78 58 d1 1c 0$ [ 815.872012] RIP [] devpts_pty_kill+0x1c/0xa0 [ 815.872012] RSP [ 815.872012] CR2: 0000000000000028 [ 815.897036] ---[ end trace eadf50b7f34e47d5 ]--- Fixes this BUG also: [ 608.366836] BUG: unable to handle kernel NULL pointer dereference at 0000000000000028 [ 608.366948] IP: [] devpts_kill_index+0x18/0x70 [ 608.367050] PGD 7c75b067 PUD 7b919067 PMD 0 [ 608.367135] Oops: 0000 [#1] PREEMPT SMP [ 608.367201] Modules linked in: kvm_intel kvm snd_hda_intel snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event microcode snd_seq psmouse snd_timer snd_seq_device serio_raw snd mac_hid soundcore snd_page_alloc rfcomm virtio_balloon parport_pc bnep bluetooth ppdev i2c_piix4 lp parport floppy [ 608.367617] CPU 2 [ 608.367669] Pid: 1918, comm: stress_test_tty Tainted: G W 3.8.0-next-20130125+ttypatch-2-xeon #2 Bochs Bochs [ 608.367796] RIP: 0010:[] [] devpts_kill_index+0x18/0x70 [ 608.367885] RSP: 0018:ffff88007ae41a88 EFLAGS: 00010286 [ 608.367951] RAX: ffffffff81417e80 RBX: ffff880036472400 RCX: 0000000180400028 [ 608.368010] RDX: ffff880036470004 RSI: 0000000000000004 RDI: 0000000000000000 [ 608.368010] RBP: ffff88007ae41a98 R08: 0000000000000000 R09: 0000000000000001 [ 608.368010] R10: ffffea0001f22e40 R11: ffffffff814151d5 R12: 0000000000000004 [ 608.368010] R13: ffff880036470000 R14: 0000000000000004 R15: ffff880036472400 [ 608.368010] FS: 00007ff7a5268700(0000) GS:ffff88007fd00000(0000) knlGS:0000000000000000 [ 608.368010] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 608.368010] CR2: 0000000000000028 CR3: 000000007a0fd000 CR4: 00000000000006e0 [ 608.368010] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 608.368010] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 608.368010] Process stress_test_tty (pid: 1918, threadinfo ffff88007ae40000, task ffff88003688dc40) [ 608.368010] Stack: [ 608.368010] ffff880036472400 0000000000000001 ffff88007ae41aa8 ffffffff81417e98 [ 608.368010] ffff88007ae41ac8 ffffffff8140c42b ffff88007ac73100 ffff88007ac73100 [ 608.368010] ffff88007ae41b98 ffffffff8140ead5 ffff88007ae41b38 ffff88007ca40e40 [ 608.368010] Call Trace: [ 608.368010] [] pty_unix98_shutdown+0x18/0x20 [ 608.368010] [] release_tty+0x3b/0xe0 [ 608.368010] [] __tty_release+0x575/0x5d0 [ 608.368010] [] ? _raw_spin_lock+0x23/0x30 [ 608.368010] [] ? _raw_spin_unlock+0x1a/0x40 [ 608.368010] [] ? __mutex_unlock_slowpath+0x48/0x60 [ 608.368010] [] tty_open+0x449/0x5f0 [ 608.368010] [] chrdev_open+0x9b/0x1c0 [ 608.368010] [] do_dentry_open+0x203/0x290 [ 608.368010] [] ? cdev_put+0x30/0x30 [ 608.368010] [] finish_open+0x35/0x50 [ 608.368010] [] do_last+0x6fe/0xe90 [ 608.368010] [] ? link_path_walk+0x7f/0x880 [ 608.368010] [] path_openat+0xbc/0x4e0 [ 608.368010] [] do_filp_open+0x41/0xa0 [ 608.368010] [] ? _raw_spin_unlock+0x1a/0x40 [ 608.368010] [] ? __alloc_fd+0xe9/0x140 [ 608.368010] [] do_sys_open+0xf4/0x1e0 [ 608.368010] [] ? _raw_spin_lock+0x23/0x30 [ 608.368010] [] sys_open+0x21/0x30 [ 608.368010] [] system_call_fastpath+0x16/0x1b [ 608.368010] Code: ec 48 83 c4 10 5b 41 5c 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 e5 48 83 ec 10 4c 89 65 f8 41 89 f4 48 89 5d f0 <48> 8b 47 28 48 81 78 58 d1 1c 00 00 74 0b 48 8b 05 4b 66 cf 00 [ 608.368010] RIP [] devpts_kill_index+0x18/0x70 [ 608.368010] RSP [ 608.368010] CR2: 0000000000000028 [ 608.394153] ---[ end trace afe83b0fb5fbda93 ]--- Reported-by: Ilya Zykov Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 755600f6120f..96dc6dd31425 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -55,7 +55,8 @@ static void pty_close(struct tty_struct *tty, struct file *filp) #ifdef CONFIG_UNIX98_PTYS if (tty->driver == ptm_driver) { mutex_lock(&devpts_mutex); - devpts_pty_kill(tty->link->driver_data); + if (tty->link->driver_data) + devpts_pty_kill(tty->link->driver_data); mutex_unlock(&devpts_mutex); } #endif @@ -703,6 +704,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) mutex_unlock(&tty_mutex); set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */ + tty->driver_data = inode; tty_add_file(tty, filp); @@ -713,14 +715,13 @@ static int ptmx_open(struct inode *inode, struct file *filp) retval = PTR_ERR(slave_inode); goto err_release; } + tty->link->driver_data = slave_inode; retval = ptm_driver->ops->open(tty, filp); if (retval) goto err_release; tty_unlock(tty); - tty->driver_data = inode; - tty->link->driver_data = slave_inode; return 0; err_release: tty_unlock(tty); -- cgit v1.2.3 From 699390354da6c258b65bf8fa79cfd5feaede50b6 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 30 Jan 2013 12:43:50 -0500 Subject: pty: Ignore slave pty close() if never successfully opened If the master and slave ptys are opened in parallel, the slave open fails because the pty is still locked. This is as designed. However, pty_close() is still called for the slave pty which sets TTY_OTHER_CLOSED in the master pty. This can cause the master open to fail as well. Use a common pattern in other tty drivers by setting TTY_IO_ERROR until the open is successful and only closing the pty if not set. Note: the master pty always closes regardless of whether the open was successful, so that proper cleanup can occur. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 96dc6dd31425..d38455fab4b7 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -38,9 +38,12 @@ static void pty_close(struct tty_struct *tty, struct file *filp) if (tty->driver->subtype == PTY_TYPE_MASTER) WARN_ON(tty->count > 1); else { + if (test_bit(TTY_IO_ERROR, &tty->flags)) + return; if (tty->count > 2) return; } + set_bit(TTY_IO_ERROR, &tty->flags); wake_up_interruptible(&tty->read_wait); wake_up_interruptible(&tty->write_wait); tty->packet = 0; @@ -246,6 +249,8 @@ static int pty_open(struct tty_struct *tty, struct file *filp) if (!tty || !tty->link) goto out; + set_bit(TTY_IO_ERROR, &tty->flags); + retval = -EIO; if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) goto out; @@ -254,6 +259,7 @@ static int pty_open(struct tty_struct *tty, struct file *filp) if (tty->link->count != 1) goto out; + clear_bit(TTY_IO_ERROR, &tty->flags); clear_bit(TTY_OTHER_CLOSED, &tty->link->flags); set_bit(TTY_THROTTLED, &tty->flags); retval = 0; -- cgit v1.2.3 From 80cace72566633bb99da1f022f71d3dac3498b02 Mon Sep 17 00:00:00 2001 From: Peter Hurley Date: Wed, 30 Jan 2013 12:43:52 -0500 Subject: pty: Ignore slave open count for master pty open Multiple slave pty opens may be performed in parallel with the master open. Of course, all the slave opens will fail because the master pty is still locked but during this time the slave pty count will be artificially greater than 1. This is should not cause the master pty open to fail. Signed-off-by: Peter Hurley Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index d38455fab4b7..c24b4db243b9 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -256,7 +256,7 @@ static int pty_open(struct tty_struct *tty, struct file *filp) goto out; if (test_bit(TTY_PTY_LOCK, &tty->link->flags)) goto out; - if (tty->link->count != 1) + if (tty->driver->subtype == PTY_TYPE_SLAVE && tty->link->count != 1) goto out; clear_bit(TTY_IO_ERROR, &tty->flags); -- cgit v1.2.3