[Yakkety,SRU] tty: Fix ldisc crash on reopened tty

Submitted by Tim Gardner on March 20, 2017, 1:52 p.m.

Details

Message ID 1490017926-11554-1-git-send-email-tim.gardner@canonical.com
State New
Headers show

Commit Message

Tim Gardner March 20, 2017, 1:52 p.m.
From: Peter Hurley <peter@hurleysoftware.com>

BugLink: http://bugs.launchpad.net/bugs/1674325

If the tty has been hungup, the ldisc instance may have been destroyed.
Continued input to the tty will be ignored as long as the ldisc instance
is not visible to the flush_to_ldisc kworker. However, when the tty
is reopened and a new ldisc instance is created, the flush_to_ldisc
kworker can obtain an ldisc reference before the new ldisc is
completely initialized. This will likely crash:

 BUG: unable to handle kernel paging request at 0000000000002260
 IP: [<ffffffff8152dc5d>] n_tty_receive_buf_common+0x6d/0xb80
 PGD 2ab581067 PUD 290c11067 PMD 0
 Oops: 0000 [#1] PREEMPT SMP
 Modules linked in: nls_iso8859_1 ip6table_filter [.....]
 CPU: 2 PID: 103 Comm: kworker/u16:1 Not tainted 4.6.0-rc7+wip-xeon+debug #rc7+wip
 Hardware name: Dell Inc. Precision WorkStation T5400  /0RW203, BIOS A11 04/30/2012
 Workqueue: events_unbound flush_to_ldisc
 task: ffff8802ad16d100 ti: ffff8802ad31c000 task.ti: ffff8802ad31c000
 RIP: 0010:[<ffffffff8152dc5d>]  [<ffffffff8152dc5d>] n_tty_receive_buf_common+0x6d/0xb80
 RSP: 0018:ffff8802ad31fc70  EFLAGS: 00010296
 RAX: 0000000000000000 RBX: ffff8802aaddd800 RCX: 0000000000000001
 RDX: 00000000ffffffff RSI: ffffffff810db48f RDI: 0000000000000246
 RBP: ffff8802ad31fd08 R08: 0000000000000000 R09: 0000000000000001
 R10: ffff8802aadddb28 R11: 0000000000000001 R12: ffff8800ba6da808
 R13: ffff8802ad18be80 R14: ffff8800ba6da858 R15: ffff8800ba6da800
 FS:  0000000000000000(0000) GS:ffff8802b0a00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000002260 CR3: 000000028ee5d000 CR4: 00000000000006e0
 Stack:
  ffffffff81531219 ffff8802aadddab8 ffff8802aadddde0 ffff8802aadddd78
  ffffffff00000001 ffff8800ba6da858 ffff8800ba6da860 ffff8802ad31fd30
  ffffffff81885f78 ffffffff81531219 0000000000000000 0000000200000000
 Call Trace:
  [<ffffffff81531219>] ? flush_to_ldisc+0x49/0xd0
  [<ffffffff81885f78>] ? mutex_lock_nested+0x2c8/0x430
  [<ffffffff81531219>] ? flush_to_ldisc+0x49/0xd0
  [<ffffffff8152e784>] n_tty_receive_buf2+0x14/0x20
  [<ffffffff81530cb2>] tty_ldisc_receive_buf+0x22/0x50
  [<ffffffff8153128e>] flush_to_ldisc+0xbe/0xd0
  [<ffffffff810a0ebd>] process_one_work+0x1ed/0x6e0
  [<ffffffff810a0e3f>] ? process_one_work+0x16f/0x6e0
  [<ffffffff810a13fe>] worker_thread+0x4e/0x490
  [<ffffffff810a13b0>] ? process_one_work+0x6e0/0x6e0
  [<ffffffff810a7ef2>] kthread+0xf2/0x110
  [<ffffffff810ae68c>] ? preempt_count_sub+0x4c/0x80
  [<ffffffff8188ab52>] ret_from_fork+0x22/0x50
  [<ffffffff810a7e00>] ? kthread_create_on_node+0x220/0x220
 Code: ff ff e8 27 a0 35 00 48 8d 83 78 05 00 00 c7 45 c0 00 00 00 00 48 89 45 80 48
       8d 83 e0 05 00 00 48 89 85 78 ff ff ff 48 8b 45 b8 <48> 8b b8 60 22 00 00 48
       8b 30 89 f8 8b 8b 88 04 00 00 29 f0 8d
 RIP  [<ffffffff8152dc5d>] n_tty_receive_buf_common+0x6d/0xb80
  RSP <ffff8802ad31fc70>
 CR2: 0000000000002260

Ensure the kworker cannot obtain the ldisc reference until the new ldisc
is completely initialized.

Fixes: 892d1fa7eaae ("tty: Destroy ldisc instance on hangup")
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from linux-next commit 71472fa9c52b1da27663c275d416d8654b905f05)
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 drivers/tty/tty_ldisc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Stefan Bader March 27, 2017, 2:11 p.m.
A bit confusing that the bug report asks for two changes and this is only one of
them...

-Stefan
Tim Gardner March 27, 2017, 2:33 p.m.
On 03/27/2017 08:11 AM, Stefan Bader wrote:
> A bit confusing that the bug report asks for two changes and this is only one of
> them...
> 
> -Stefan
> 
> 
> 

One of the patches mentioned in the bug report had already been applied.
Thadeu Lima de Souza Cascardo March 29, 2017, 2:55 p.m.
On Mon, Mar 27, 2017 at 08:33:13AM -0600, Tim Gardner wrote:
> On 03/27/2017 08:11 AM, Stefan Bader wrote:
> > A bit confusing that the bug report asks for two changes and this is only one of
> > them...
> > 
> > -Stefan
> > 
> > 
> > 
> 
> One of the patches mentioned in the bug report had already been applied.
> 

To which branch or which commit id are you referring to? I can't find it in
yakkety master-next.

Thanks.
Cascardo.

> -- 
> Tim Gardner tim.gardner@canonical.com
>
Tim Gardner March 29, 2017, 3:31 p.m.
Whoops, I did miss a patch.

[PATCH 1/2] tty: Fix ldisc crash on reopened tty
[PATCH 2/2] UBUNTU: SAUCE: powerpc/powernv/cpuidle: Pass correct

rtg
Brad Figg March 29, 2017, 10:51 p.m.

Stefan Bader March 30, 2017, 2:20 p.m.
On 29.03.2017 17:31, Tim Gardner wrote:
> Whoops, I did miss a patch.
> 
> [PATCH 1/2] tty: Fix ldisc crash on reopened tty
> [PATCH 2/2] UBUNTU: SAUCE: powerpc/powernv/cpuidle: Pass correct
> 
> rtg
> 
>
Thadeu Lima de Souza Cascardo March 30, 2017, 7:28 p.m.
Applied to yakkety master-next branch.

Thanks.
Cascardo.

Patch hide | download patch | download mbox

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 68947f6..4ee7742 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -669,16 +669,17 @@  int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 		tty_ldisc_put(tty->ldisc);
 	}
 
-	/* switch the line discipline */
-	tty->ldisc = ld;
 	tty_set_termios_ldisc(tty, disc);
-	retval = tty_ldisc_open(tty, tty->ldisc);
+	retval = tty_ldisc_open(tty, ld);
 	if (retval) {
 		if (!WARN_ON(disc == N_TTY)) {
-			tty_ldisc_put(tty->ldisc);
-			tty->ldisc = NULL;
+			tty_ldisc_put(ld);
+			ld = NULL;
 		}
 	}
+
+	/* switch the line discipline */
+	smp_store_release(&tty->ldisc, ld);
 	return retval;
 }