linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KASAN: user-memory-access Write in n_tty_set_termios
@ 2018-04-01 17:01 syzbot
  2018-04-05 10:31 ` Tetsuo Handa
  0 siblings, 1 reply; 10+ messages in thread
From: syzbot @ 2018-04-01 17:01 UTC (permalink / raw)
  To: gregkh, jslaby, linux-kernel, syzkaller-bugs

Hello,

syzbot hit the following crash on upstream commit
10b84daddbec72c6b440216a69de9a9605127f7a (Sat Mar 31 17:59:00 2018 +0000)
Merge branch 'perf-urgent-for-linus' of  
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=3aa9784721dfb90e984d

So far this crash happened 2 times on upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5259401455730688
syzkaller reproducer:  
https://syzkaller.appspot.com/x/repro.syz?id=5604984590696448
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=6343571460325376
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=-2760467897697295172
compiler: gcc (GCC) 7.1.1 20170620

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+3aa9784721dfb90e984d@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.
If you forward the report, please keep this part and the footer.

==================================================================
BUG: KASAN: user-memory-access in memset include/linux/string.h:330 [inline]
BUG: KASAN: user-memory-access in bitmap_zero include/linux/bitmap.h:208  
[inline]
BUG: KASAN: user-memory-access in n_tty_set_termios+0xfc/0xcf0  
drivers/tty/n_tty.c:1766
Write of size 512 at addr 0000000000001060 by task syzkaller311122/4421

CPU: 0 PID: 4421 Comm: syzkaller311122 Not tainted 4.16.0-rc7+ #9
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x194/0x24d lib/dump_stack.c:53
  kasan_report_error mm/kasan/report.c:352 [inline]
  kasan_report+0x140/0x360 mm/kasan/report.c:412
  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
  check_memory_region+0x137/0x190 mm/kasan/kasan.c:267
  memset+0x23/0x40 mm/kasan/kasan.c:285
  memset include/linux/string.h:330 [inline]
  bitmap_zero include/linux/bitmap.h:208 [inline]
  n_tty_set_termios+0xfc/0xcf0 drivers/tty/n_tty.c:1766
  tty_set_termios+0x750/0xa60 drivers/tty/tty_ioctl.c:341
  set_termios+0x392/0x6d0 drivers/tty/tty_ioctl.c:414
  tty_mode_ioctl+0x9fc/0xb30 drivers/tty/tty_ioctl.c:749
  n_tty_ioctl_helper+0x40/0x360 drivers/tty/tty_ioctl.c:940
  n_tty_ioctl+0x14d/0x2d0 drivers/tty/n_tty.c:2441
  tty_ioctl+0x336/0x1610 drivers/tty/tty_io.c:2655
  vfs_ioctl fs/ioctl.c:46 [inline]
  do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
  SYSC_ioctl fs/ioctl.c:701 [inline]
  SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x445bf9
RSP: 002b:00007fdf68f52d18 EFLAGS: 00000297 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000006dac24 RCX: 0000000000445bf9
RDX: 0000000020000040 RSI: 0000000000005402 RDI: 0000000000000021
RBP: 00000000006dac20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000297 R12: 6d74702f7665642f
R13: 00007ffdda32610f R14: 00007fdf68f539c0 R15: 0000000000000007
==================================================================
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 4421 Comm: syzkaller311122 Tainted: G    B             
4.16.0-rc7+ #9
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x194/0x24d lib/dump_stack.c:53
  panic+0x1e4/0x41c kernel/panic.c:183
  kasan_end_report+0x50/0x50 mm/kasan/report.c:180
  kasan_report_error mm/kasan/report.c:359 [inline]
  kasan_report+0x149/0x360 mm/kasan/report.c:412
  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
  check_memory_region+0x137/0x190 mm/kasan/kasan.c:267
  memset+0x23/0x40 mm/kasan/kasan.c:285
  memset include/linux/string.h:330 [inline]
  bitmap_zero include/linux/bitmap.h:208 [inline]
  n_tty_set_termios+0xfc/0xcf0 drivers/tty/n_tty.c:1766
  tty_set_termios+0x750/0xa60 drivers/tty/tty_ioctl.c:341
  set_termios+0x392/0x6d0 drivers/tty/tty_ioctl.c:414
  tty_mode_ioctl+0x9fc/0xb30 drivers/tty/tty_ioctl.c:749
  n_tty_ioctl_helper+0x40/0x360 drivers/tty/tty_ioctl.c:940
  n_tty_ioctl+0x14d/0x2d0 drivers/tty/n_tty.c:2441
  tty_ioctl+0x336/0x1610 drivers/tty/tty_io.c:2655
  vfs_ioctl fs/ioctl.c:46 [inline]
  do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
  SYSC_ioctl fs/ioctl.c:701 [inline]
  SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x445bf9
RSP: 002b:00007fdf68f52d18 EFLAGS: 00000297 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000006dac24 RCX: 0000000000445bf9
RDX: 0000000020000040 RSI: 0000000000005402 RDI: 0000000000000021
RBP: 00000000006dac20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000297 R12: 6d74702f7665642f
R13: 00007ffdda32610f R14: 00007fdf68f539c0 R15: 0000000000000007
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.
Note: all commands must start from beginning of the line in the email body.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: user-memory-access Write in n_tty_set_termios
  2018-04-01 17:01 KASAN: user-memory-access Write in n_tty_set_termios syzbot
@ 2018-04-05 10:31 ` Tetsuo Handa
  2018-04-06 10:06   ` Tetsuo Handa
  0 siblings, 1 reply; 10+ messages in thread
From: Tetsuo Handa @ 2018-04-05 10:31 UTC (permalink / raw)
  To: gregkh, jslaby; +Cc: syzbot, linux-kernel, syzkaller-bugs

Hello.

I manually simplified the reproducer. Since the bug is timing dependent,
this reproducer might fail to reproducer the bug. Anyway, I guess that
there is a race condition between

	vfree(ldata);
	tty->disc_data = NULL;

at n_tty_close() by something (ioctl(TIOCVHANGUP) ?) and

	struct n_tty_data *ldata = tty->disc_data;

	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & (ICANON | EXTPROC)) {
		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);

at n_tty_set_termios() by ioctl(TCSETS), for the report says that ldata == NULL
("Write of size 512 at addr 0000000000001060").

----------------------------------------
#include <fcntl.h>
#include <linux/futex.h>
#include <pthread.h>
#include <stdio.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <termios.h>
#define TIOCGPTPEER     _IO('T', 0x41) 

static const int zero = 0;
static int master_fd = EOF;
static int slave_fd = EOF;
static void execute_call(int call)
{
	struct termios term;
	char buf[128];
	int ptyno = 0;
	switch (call) {
	case 0:
		master_fd = open("/dev/ptmx", O_RDONLY);
		ioctl(master_fd, TIOCSPTLCK, &zero);
		if (ioctl(master_fd, TIOCGPTN, &ptyno))
			break;
		sprintf(buf, "/dev/pts/%d", ptyno);
		slave_fd = open(buf, O_RDONLY, 0);
		usleep(5 * 1000);
		ioctl(slave_fd, TIOCVHANGUP, 0);
		break;
	case 5:
		memset(&term, 0, sizeof(term));
		ioctl(master_fd, TCSETS, &term);
		break;
	case 6:
		ioctl(master_fd, TIOCGPTPEER, 0);
		break;
	}
}

struct thread_t {
	int created, running, call;
	pthread_t th;
};

static struct thread_t threads[16];
static int running;
static int collide;

static void* thr(void* arg)
{
	struct thread_t* th = (struct thread_t*)arg;
	for (;;) {
		while (!__atomic_load_n(&th->running, __ATOMIC_ACQUIRE))
			syscall(SYS_futex, &th->running, FUTEX_WAIT, 0, 0);
		execute_call(th->call);
		__atomic_fetch_sub(&running, 1, __ATOMIC_RELAXED);
		__atomic_store_n(&th->running, 0, __ATOMIC_RELEASE);
		syscall(SYS_futex, &th->running, FUTEX_WAKE);
	}
	return 0;
}

static void execute(int num_calls)
{
	int call, thread;
	running = 0;
	for (call = 0; call < num_calls; call++) {
		for (thread = 0; thread < sizeof(threads) / sizeof(threads[0]); thread++) {
			struct thread_t* th = &threads[thread];
			if (!th->created) {
				th->created = 1;
				pthread_attr_t attr;
				pthread_attr_init(&attr);
				pthread_attr_setstacksize(&attr, 128 << 10);
				pthread_create(&th->th, &attr, thr, th);
			}
			if (!__atomic_load_n(&th->running, __ATOMIC_ACQUIRE)) {
				th->call = call;
				__atomic_fetch_add(&running, 1, __ATOMIC_RELAXED);
				__atomic_store_n(&th->running, 1, __ATOMIC_RELEASE);
				syscall(SYS_futex, &th->running, FUTEX_WAKE);
				if (collide && call % 2)
					break;
				struct timespec ts;
				ts.tv_sec = 0;
				ts.tv_nsec = 20 * 1000 * 1000;
				syscall(SYS_futex, &th->running, FUTEX_WAIT, 1, &ts);
				if (running)
					usleep((call == num_calls - 1) ? 10000 : 1000);
				break;
			}
		}
	}
}

int main(int argc, char *argv[])
{
	while (1) {
		execute(7);
		collide = 1;
		execute(7);
	}
	return 0;
}
----------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: KASAN: user-memory-access Write in n_tty_set_termios
  2018-04-05 10:31 ` Tetsuo Handa
@ 2018-04-06 10:06   ` Tetsuo Handa
  2018-07-17 10:14     ` [PATCH] n_tty: Protect tty->disc_data using refcount Tetsuo Handa
  0 siblings, 1 reply; 10+ messages in thread
From: Tetsuo Handa @ 2018-04-06 10:06 UTC (permalink / raw)
  To: gregkh, jslaby; +Cc: syzbot, linux-kernel, syzkaller-bugs

On 2018/04/05 19:31, Tetsuo Handa wrote:
> Hello.
> 
> I manually simplified the reproducer. Since the bug is timing dependent,
> this reproducer might fail to reproducer the bug. Anyway, I guess that
> there is a race condition between
> 
> 	vfree(ldata);
> 	tty->disc_data = NULL;
> 
> at n_tty_close() by something (ioctl(TIOCVHANGUP) ?) and
> 
> 	struct n_tty_data *ldata = tty->disc_data;
> 
> 	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & (ICANON | EXTPROC)) {
> 		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
> 
> at n_tty_set_termios() by ioctl(TCSETS), for the report says that ldata == NULL
> ("Write of size 512 at addr 0000000000001060").

I confirmed that ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
We need some lock for protecting tty->disc_data.

--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1861,6 +1861,7 @@ static void n_tty_close(struct tty_struct *tty)

        vfree(ldata);
        tty->disc_data = NULL;
+       WARN_ON(1);
 }

 /**

ioctl(TIOCVHANGUP):
[   75.176171] RIP: 0010:n_tty_close+0x34/0x40   /* drivers/tty/n_tty.c:1863 */
[   75.176193] Call Trace:
[   75.176200]  tty_ldisc_kill+0x15/0x30         /* drivers/tty/tty_ldisc.c:641 */
[   75.176204]  tty_ldisc_hangup+0xfa/0x1c0      /* drivers/tty/tty_ldisc.c:758 */
[   75.176209]  __tty_hangup.part.26+0x16b/0x280 /* drivers/tty/tty_io.c:623 */
[   75.176216]  tty_ioctl+0x61f/0x950            /* drivers/tty/tty_io.c:2585 */
[   75.176244]  do_vfs_ioctl+0xa7/0x6d0          /* fs/ioctl.c:686 */
[   75.176257]  ksys_ioctl+0x6b/0x80             /* fs/ioctl.c:701 */
[   75.176266]  SyS_ioctl+0x5/0x10               /* fs/ioctl.c:706 */
[   75.176268]  do_syscall_64+0x6e/0x270         /* arch/x86/entry/common.c:287 */

ioctl(TCSETS):
[   75.783367] BUG: unable to handle kernel paging request at 0000000000001060
[   75.934024] RIP: 0010:n_tty_set_termios+0x17c/0x2a0 /* drivers/tty/n_tty.c:1766 */
[   75.950549] Call Trace:
[   75.952733]  tty_set_termios+0x18a/0x200            /* drivers/tty/tty_ioctl.c:342 */
[   75.954915]  set_termios+0x14a/0x250                /* drivers/tty/tty_ioctl.c:414 */
[   75.956823]  tty_mode_ioctl+0x42e/0x4f0             /* drivers/tty/tty_ioctl.c:749 */
[   75.958632]  tty_ioctl+0x128/0x950                  /* drivers/tty/tty_io.c:2655 */
[   75.968883]  do_vfs_ioctl+0xa7/0x6d0                /* fs/ioctl.c:686 */
[   75.972892]  ksys_ioctl+0x6b/0x80                   /* fs/ioctl.c:701 */
[   75.977031]  SyS_ioctl+0x5/0x10                     /* fs/ioctl.c:706 */
[   75.977036]  do_syscall_64+0x6e/0x270               /* arch/x86/entry/common.c:287 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] n_tty: Protect tty->disc_data using refcount.
  2018-04-06 10:06   ` Tetsuo Handa
@ 2018-07-17 10:14     ` Tetsuo Handa
  2018-07-24 15:22       ` [PATCH v2] " Tetsuo Handa
  0 siblings, 1 reply; 10+ messages in thread
From: Tetsuo Handa @ 2018-07-17 10:14 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: syzbot, linux-kernel, syzkaller-bugs, Alexander Viro, linux-fsdevel

From e8360c16fd07985686bcfb388364103f35a6523a Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 17 Jul 2018 16:43:32 +0900
Subject: [PATCH] n_tty: Protect tty->disc_data using refcount.

syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.

Since we don't want to introduce new locking dependency, this patch
converts "struct n_tty_data *ldata = tty->disc_data;" in individual
function into a function argument which follows "struct tty *", and
holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
in order to ensure that memory which contains "struct n_tty_data" will
not be released while processing individual function.

What is the best return value for these "struct tty_ldisc_ops" hooks
when we hit this race is not clear. We might need to tweak some of them.

[1] https://syzkaller.appspot.com/bug?id=1e850009fca0b64ce49dc16499bda4f7de0ab1a5

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/tty/n_tty.c | 503 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 308 insertions(+), 195 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4317422..fc0a4bf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -122,6 +122,10 @@ struct n_tty_data {
 
 	struct mutex atomic_read_lock;
 	struct mutex output_lock;
+
+	/* Race protection. */
+	refcount_t users;
+	struct rcu_head rcu;
 };
 
 #define MASK(x) ((x) & (N_TTY_BUF_SIZE - 1))
@@ -152,10 +156,9 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 	return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
 }
 
-static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
-			    size_t tail, size_t n)
+static int tty_copy_to_user(struct tty_struct *tty, struct n_tty_data *ldata,
+			    void __user *to, size_t tail, size_t n)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t size = N_TTY_BUF_SIZE - tail;
 	const void *from = read_buf_addr(ldata, tail);
 	int uncopied;
@@ -186,10 +189,8 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
  *		holds non-exclusive termios_rwsem
  */
 
-static void n_tty_kick_worker(struct tty_struct *tty)
+static void n_tty_kick_worker(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	/* Did the input worker stop? Restart it */
 	if (unlikely(ldata->no_room)) {
 		ldata->no_room = 0;
@@ -206,9 +207,8 @@ static void n_tty_kick_worker(struct tty_struct *tty)
 	}
 }
 
-static ssize_t chars_in_buffer(struct tty_struct *tty)
+static ssize_t chars_in_buffer(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	ssize_t n = 0;
 
 	if (!ldata->icanon)
@@ -233,10 +233,9 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
 	kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
 }
 
-static void n_tty_check_throttle(struct tty_struct *tty)
+static void n_tty_check_throttle(struct tty_struct *tty,
+				 struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	/*
 	 * Check the remaining room for the input canonicalization
 	 * mode.  We don't want to throttle the driver if we're in
@@ -257,12 +256,13 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 	__tty_set_flow_change(tty, 0);
 }
 
-static void n_tty_check_unthrottle(struct tty_struct *tty)
+static void n_tty_check_unthrottle(struct tty_struct *tty,
+				   struct n_tty_data *ldata)
 {
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
-		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+		if (chars_in_buffer(tty, ldata) > TTY_THRESHOLD_UNTHROTTLE)
 			return;
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 		tty_wakeup(tty->link);
 		return;
 	}
@@ -278,9 +278,9 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 	while (1) {
 		int unthrottled;
 		tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
-		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+		if (chars_in_buffer(tty, ldata) > TTY_THRESHOLD_UNTHROTTLE)
 			break;
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 		unthrottled = tty_unthrottle_safe(tty);
 		if (!unthrottled)
 			break;
@@ -353,11 +353,12 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
  *	Locking: ctrl_lock, exclusive termios_rwsem
  */
 
-static void n_tty_flush_buffer(struct tty_struct *tty)
+static void __n_tty_flush_buffer(struct tty_struct *tty,
+				 struct n_tty_data *ldata)
 {
 	down_write(&tty->termios_rwsem);
-	reset_buffer_flags(tty->disc_data);
-	n_tty_kick_worker(tty);
+	reset_buffer_flags(ldata);
+	n_tty_kick_worker(tty, ldata);
 
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
@@ -413,9 +414,9 @@ static inline int is_continuation(unsigned char c, struct tty_struct *tty)
  *		 the column state and space left in the buffer
  */
 
-static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
+static int do_output_char(unsigned char c, struct tty_struct *tty,
+			  struct n_tty_data *ldata, int space)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	spaces;
 
 	if (!space)
@@ -488,15 +489,15 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
  *		  tty layer write lock)
  */
 
-static int process_output(unsigned char c, struct tty_struct *tty)
+static int process_output(unsigned char c, struct tty_struct *tty,
+			  struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space, retval;
 
 	mutex_lock(&ldata->output_lock);
 
 	space = tty_write_room(tty);
-	retval = do_output_char(c, tty, space);
+	retval = do_output_char(c, tty, ldata, space);
 
 	mutex_unlock(&ldata->output_lock);
 	if (retval < 0)
@@ -525,9 +526,9 @@ static int process_output(unsigned char c, struct tty_struct *tty)
  */
 
 static ssize_t process_output_block(struct tty_struct *tty,
+				    struct n_tty_data *ldata,
 				    const unsigned char *buf, unsigned int nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space;
 	int	i;
 	const unsigned char *cp;
@@ -608,9 +609,8 @@ static ssize_t process_output_block(struct tty_struct *tty,
  *	Locking: callers must hold output_lock
  */
 
-static size_t __process_echoes(struct tty_struct *tty)
+static size_t __process_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space, old_space;
 	size_t tail;
 	unsigned char c;
@@ -721,7 +721,8 @@ static size_t __process_echoes(struct tty_struct *tty)
 				break;
 		} else {
 			if (O_OPOST(tty)) {
-				int retval = do_output_char(c, tty, space);
+				int retval = do_output_char(c, tty, ldata,
+							    space);
 				if (retval < 0)
 					break;
 				space -= retval;
@@ -754,9 +755,8 @@ static size_t __process_echoes(struct tty_struct *tty)
 	return old_space - space;
 }
 
-static void commit_echoes(struct tty_struct *tty)
+static void commit_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t nr, old, echoed;
 	size_t head;
 
@@ -776,16 +776,15 @@ static void commit_echoes(struct tty_struct *tty)
 	}
 
 	ldata->echo_commit = head;
-	echoed = __process_echoes(tty);
+	echoed = __process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 
 	if (echoed && tty->ops->flush_chars)
 		tty->ops->flush_chars(tty);
 }
 
-static void process_echoes(struct tty_struct *tty)
+static void process_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
 	if (ldata->echo_mark == ldata->echo_tail)
@@ -793,7 +792,7 @@ static void process_echoes(struct tty_struct *tty)
 
 	mutex_lock(&ldata->output_lock);
 	ldata->echo_commit = ldata->echo_mark;
-	echoed = __process_echoes(tty);
+	echoed = __process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 
 	if (echoed && tty->ops->flush_chars)
@@ -801,17 +800,15 @@ static void process_echoes(struct tty_struct *tty)
 }
 
 /* NB: echo_mark and echo_head should be equivalent here */
-static void flush_echoes(struct tty_struct *tty)
+static void flush_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
 	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);
 	ldata->echo_commit = ldata->echo_head;
-	__process_echoes(tty);
+	__process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 }
 
@@ -921,10 +918,9 @@ static void echo_char_raw(unsigned char c, struct n_tty_data *ldata)
  *	(where X is the letter representing the control char).
  */
 
-static void echo_char(unsigned char c, struct tty_struct *tty)
+static void echo_char(unsigned char c, struct tty_struct *tty,
+		      struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (c == ECHO_OP_START) {
 		add_echo_byte(ECHO_OP_START, ldata);
 		add_echo_byte(ECHO_OP_START, ldata);
@@ -961,9 +957,9 @@ static inline void finish_erasing(struct n_tty_data *ldata)
  *		caller holds non-exclusive termios_rwsem
  */
 
-static void eraser(unsigned char c, struct tty_struct *tty)
+static void eraser(unsigned char c, struct tty_struct *tty,
+		   struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	enum { ERASE, WERASE, KILL } kill_type;
 	size_t head;
 	size_t cnt;
@@ -985,7 +981,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 		if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
 			ldata->read_head = ldata->canon_head;
 			finish_erasing(ldata);
-			echo_char(KILL_CHAR(tty), tty);
+			echo_char(KILL_CHAR(tty), tty, ldata);
 			/* Add a newline if ECHOK is on and ECHOKE is off. */
 			if (L_ECHOK(tty))
 				echo_char_raw('\n', ldata);
@@ -1025,14 +1021,14 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 					ldata->erasing = 1;
 				}
 				/* if cnt > 1, output a multi-byte character */
-				echo_char(c, tty);
+				echo_char(c, tty, ldata);
 				while (--cnt > 0) {
 					head++;
 					echo_char_raw(read_buf(ldata, head), ldata);
 					echo_move_back_col(ldata);
 				}
 			} else if (kill_type == ERASE && !L_ECHOE(tty)) {
-				echo_char(ERASE_CHAR(tty), tty);
+				echo_char(ERASE_CHAR(tty), tty, ldata);
 			} else if (c == '\t') {
 				unsigned int num_chars = 0;
 				int after_tab = 0;
@@ -1103,10 +1099,8 @@ static void __isig(int sig, struct tty_struct *tty)
 	}
 }
 
-static void isig(int sig, struct tty_struct *tty)
+static void isig(int sig, struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (L_NOFLSH(tty)) {
 		/* signal only */
 		__isig(sig, tty);
@@ -1127,7 +1121,7 @@ static void isig(int sig, struct tty_struct *tty)
 		tty_driver_flush_buffer(tty);
 
 		/* clear input buffer */
-		reset_buffer_flags(tty->disc_data);
+		reset_buffer_flags(ldata);
 
 		/* notify pty master of flush */
 		if (tty->link)
@@ -1151,14 +1145,13 @@ static void isig(int sig, struct tty_struct *tty)
  *	Note: may get exclusive termios_rwsem if flushing input buffer
  */
 
-static void n_tty_receive_break(struct tty_struct *tty)
+static void n_tty_receive_break(struct tty_struct *tty,
+				struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_IGNBRK(tty))
 		return;
 	if (I_BRKINT(tty)) {
-		isig(SIGINT, tty);
+		isig(SIGINT, tty, ldata);
 		return;
 	}
 	if (I_PARMRK(tty)) {
@@ -1181,10 +1174,9 @@ static void n_tty_receive_break(struct tty_struct *tty)
  *	private.
  */
 
-static void n_tty_receive_overrun(struct tty_struct *tty)
+static void n_tty_receive_overrun(struct tty_struct *tty,
+				  struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	ldata->num_overrun++;
 	if (time_after(jiffies, ldata->overrun_time + HZ) ||
 			time_after(ldata->overrun_time, jiffies)) {
@@ -1205,10 +1197,10 @@ static void n_tty_receive_overrun(struct tty_struct *tty)
  *	n_tty_receive_buf()/producer path:
  *		caller holds non-exclusive termios_rwsem
  */
-static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_parity_error(struct tty_struct *tty,
+				       struct n_tty_data *ldata,
+				       unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_INPCK(tty)) {
 		if (I_IGNPAR(tty))
 			return;
@@ -1223,16 +1215,17 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
+n_tty_receive_signal_char(struct tty_struct *tty, struct n_tty_data *ldata,
+			  int signal, unsigned char c)
 {
-	isig(signal, tty);
+	isig(signal, tty, ldata);
 	if (I_IXON(tty))
 		start_tty(tty);
 	if (L_ECHO(tty)) {
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	} else
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	return;
 }
 
@@ -1253,14 +1246,13 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
  */
 
 static int
-n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_special(struct tty_struct *tty, struct n_tty_data *ldata,
+			   unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_IXON(tty)) {
 		if (c == START_CHAR(tty)) {
 			start_tty(tty);
-			process_echoes(tty);
+			process_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == STOP_CHAR(tty)) {
@@ -1271,20 +1263,20 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 
 	if (L_ISIG(tty)) {
 		if (c == INTR_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGINT, c);
+			n_tty_receive_signal_char(tty, ldata, SIGINT, c);
 			return 0;
 		} else if (c == QUIT_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGQUIT, c);
+			n_tty_receive_signal_char(tty, ldata, SIGQUIT, c);
 			return 0;
 		} else if (c == SUSP_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGTSTP, c);
+			n_tty_receive_signal_char(tty, ldata, SIGTSTP, c);
 			return 0;
 		}
 	}
 
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 
 	if (c == '\r') {
@@ -1298,8 +1290,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 	if (ldata->icanon) {
 		if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
 		    (c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
-			eraser(c, tty);
-			commit_echoes(tty);
+			eraser(c, tty, ldata);
+			commit_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
@@ -1309,7 +1301,7 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 				if (L_ECHOCTL(tty)) {
 					echo_char_raw('^', ldata);
 					echo_char_raw('\b', ldata);
-					commit_echoes(tty);
+					commit_echoes(tty, ldata);
 				}
 			}
 			return 1;
@@ -1318,19 +1310,19 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 			size_t tail = ldata->canon_head;
 
 			finish_erasing(ldata);
-			echo_char(c, tty);
+			echo_char(c, tty, ldata);
 			echo_char_raw('\n', ldata);
 			while (MASK(tail) != MASK(ldata->read_head)) {
-				echo_char(read_buf(ldata, tail), tty);
+				echo_char(read_buf(ldata, tail), tty, ldata);
 				tail++;
 			}
-			commit_echoes(tty);
+			commit_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == '\n') {
 			if (L_ECHO(tty) || L_ECHONL(tty)) {
 				echo_char_raw('\n', ldata);
-				commit_echoes(tty);
+				commit_echoes(tty, ldata);
 			}
 			goto handle_newline;
 		}
@@ -1347,8 +1339,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 				/* Record the column of first canon char. */
 				if (ldata->canon_head == ldata->read_head)
 					echo_set_canon_col(ldata);
-				echo_char(c, tty);
-				commit_echoes(tty);
+				echo_char(c, tty, ldata);
+				commit_echoes(tty, ldata);
 			}
 			/*
 			 * XXX does PARMRK doubling happen for
@@ -1375,9 +1367,9 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 			/* Record the column of first canon char. */
 			if (ldata->canon_head == ldata->read_head)
 				echo_set_canon_col(ldata);
-			echo_char(c, tty);
+			echo_char(c, tty, ldata);
 		}
-		commit_echoes(tty);
+		commit_echoes(tty, ldata);
 	}
 
 	/* PARMRK doubling check */
@@ -1389,21 +1381,20 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 }
 
 static inline void
-n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_inline(struct tty_struct *tty, struct n_tty_data *ldata,
+			  unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 	if (L_ECHO(tty)) {
 		finish_erasing(ldata);
 		/* Record the column of first canon char. */
 		if (ldata->canon_head == ldata->read_head)
 			echo_set_canon_col(ldata);
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	}
 	/* PARMRK doubling check */
 	if (c == (unsigned char) '\377' && I_PARMRK(tty))
@@ -1411,32 +1402,34 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char(struct tty_struct *tty, struct n_tty_data *ldata,
+			       unsigned char c)
 {
-	n_tty_receive_char_inline(tty, c);
+	n_tty_receive_char_inline(tty, ldata, c);
 }
 
 static inline void
-n_tty_receive_char_fast(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_fast(struct tty_struct *tty, struct n_tty_data *ldata,
+			unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 	if (L_ECHO(tty)) {
 		finish_erasing(ldata);
 		/* Record the column of first canon char. */
 		if (ldata->canon_head == ldata->read_head)
 			echo_set_canon_col(ldata);
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	}
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_closing(struct tty_struct *tty,
+				       struct n_tty_data *ldata,
+				       unsigned char c)
 {
 	if (I_ISTRIP(tty))
 		c &= 0x7f;
@@ -1451,24 +1444,25 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 			  c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
 			  c != SUSP_CHAR(tty))) {
 			start_tty(tty);
-			process_echoes(tty);
+			process_echoes(tty, ldata);
 		}
 	}
 }
 
 static void
-n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_flagged(struct tty_struct *tty, struct n_tty_data *ldata,
+			   unsigned char c, char flag)
 {
 	switch (flag) {
 	case TTY_BREAK:
-		n_tty_receive_break(tty);
+		n_tty_receive_break(tty, ldata);
 		break;
 	case TTY_PARITY:
 	case TTY_FRAME:
-		n_tty_receive_parity_error(tty, c);
+		n_tty_receive_parity_error(tty, ldata, c);
 		break;
 	case TTY_OVERRUN:
-		n_tty_receive_overrun(tty);
+		n_tty_receive_overrun(tty, ldata);
 		break;
 	default:
 		tty_err(tty, "unknown flag %d\n", flag);
@@ -1477,26 +1471,24 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_lnext(struct tty_struct *tty, struct n_tty_data *ldata,
+			 unsigned char c, char flag)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	ldata->lnext = 0;
 	if (likely(flag == TTY_NORMAL)) {
 		if (I_ISTRIP(tty))
 			c &= 0x7f;
 		if (I_IUCLC(tty) && L_IEXTEN(tty))
 			c = tolower(c);
-		n_tty_receive_char(tty, c);
+		n_tty_receive_char(tty, ldata, c);
 	} else
-		n_tty_receive_char_flagged(tty, c, flag);
+		n_tty_receive_char_flagged(tty, ldata, c, flag);
 }
 
 static void
-n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
-			   char *fp, int count)
+n_tty_receive_buf_real_raw(struct tty_struct *tty, struct n_tty_data *ldata,
+			   const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
 
 	head = ldata->read_head & (N_TTY_BUF_SIZE - 1);
@@ -1513,10 +1505,9 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_buf_raw(struct tty_struct *tty, const unsigned char *cp,
-		      char *fp, int count)
+n_tty_receive_buf_raw(struct tty_struct *tty, struct n_tty_data *ldata,
+		      const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1525,13 +1516,13 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 		if (likely(flag == TTY_NORMAL))
 			put_tty_queue(*cp++, ldata);
 		else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
 static void
-n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+n_tty_receive_buf_closing(struct tty_struct *tty, struct n_tty_data *ldata,
+			  const unsigned char *cp, char *fp, int count)
 {
 	char flag = TTY_NORMAL;
 
@@ -1539,15 +1530,14 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 		if (fp)
 			flag = *fp++;
 		if (likely(flag == TTY_NORMAL))
-			n_tty_receive_char_closing(tty, *cp++);
+			n_tty_receive_char_closing(tty, ldata, *cp++);
 	}
 }
 
 static void
-n_tty_receive_buf_standard(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+n_tty_receive_buf_standard(struct tty_struct *tty, struct n_tty_data *ldata,
+			   const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1565,23 +1555,24 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 				continue;
 			}
 			if (!test_bit(c, ldata->char_map))
-				n_tty_receive_char_inline(tty, c);
-			else if (n_tty_receive_char_special(tty, c) && count) {
+				n_tty_receive_char_inline(tty, ldata, c);
+			else if (n_tty_receive_char_special(tty, ldata, c) &&
+				 count) {
 				if (fp)
 					flag = *fp++;
-				n_tty_receive_char_lnext(tty, *cp++, flag);
+				n_tty_receive_char_lnext(tty, ldata, *cp++,
+							 flag);
 				count--;
 			}
 		} else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
 static void
-n_tty_receive_buf_fast(struct tty_struct *tty, const unsigned char *cp,
-		       char *fp, int count)
+n_tty_receive_buf_fast(struct tty_struct *tty, struct n_tty_data *ldata,
+		       const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1591,46 +1582,47 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 			unsigned char c = *cp++;
 
 			if (!test_bit(c, ldata->char_map))
-				n_tty_receive_char_fast(tty, c);
-			else if (n_tty_receive_char_special(tty, c) && count) {
+				n_tty_receive_char_fast(tty, ldata, c);
+			else if (n_tty_receive_char_special(tty, ldata, c) &&
+				 count) {
 				if (fp)
 					flag = *fp++;
-				n_tty_receive_char_lnext(tty, *cp++, flag);
+				n_tty_receive_char_lnext(tty, ldata, *cp++,
+							 flag);
 				count--;
 			}
 		} else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
-static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+static void __receive_buf(struct tty_struct *tty, struct n_tty_data *ldata,
+			  const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
 
 	if (ldata->real_raw)
-		n_tty_receive_buf_real_raw(tty, cp, fp, count);
+		n_tty_receive_buf_real_raw(tty, ldata, cp, fp, count);
 	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
-		n_tty_receive_buf_raw(tty, cp, fp, count);
+		n_tty_receive_buf_raw(tty, ldata, cp, fp, count);
 	else if (tty->closing && !L_EXTPROC(tty))
-		n_tty_receive_buf_closing(tty, cp, fp, count);
+		n_tty_receive_buf_closing(tty, ldata, cp, fp, count);
 	else {
 		if (ldata->lnext) {
 			char flag = TTY_NORMAL;
 
 			if (fp)
 				flag = *fp++;
-			n_tty_receive_char_lnext(tty, *cp++, flag);
+			n_tty_receive_char_lnext(tty, ldata, *cp++, flag);
 			count--;
 		}
 
 		if (!preops && !I_PARMRK(tty))
-			n_tty_receive_buf_fast(tty, cp, fp, count);
+			n_tty_receive_buf_fast(tty, ldata, cp, fp, count);
 		else
-			n_tty_receive_buf_standard(tty, cp, fp, count);
+			n_tty_receive_buf_standard(tty, ldata, cp, fp, count);
 
-		flush_echoes(tty);
+		flush_echoes(tty, ldata);
 		if (tty->ops->flush_chars)
 			tty->ops->flush_chars(tty);
 	}
@@ -1681,10 +1673,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
  *		publishes commit_head or canon_head
  */
 static int
-n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
-			 char *fp, int count, int flow)
+n_tty_receive_buf_common(struct tty_struct *tty, struct n_tty_data *ldata,
+			 const unsigned char *cp, char *fp, int count, int flow)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int room, n, rcvd = 0, overflow;
 
 	down_read(&tty->termios_rwsem);
@@ -1724,7 +1715,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 		/* ignore parity errors if handling overflow */
 		if (!overflow || !fp || *fp != TTY_PARITY)
-			__receive_buf(tty, cp, fp, n);
+			__receive_buf(tty, ldata, cp, fp, n);
 
 		cp += n;
 		if (fp)
@@ -1743,23 +1734,25 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			__tty_set_flow_change(tty, 0);
 		}
 	} else
-		n_tty_check_throttle(tty);
+		n_tty_check_throttle(tty, ldata);
 
 	up_read(&tty->termios_rwsem);
 
 	return rcvd;
 }
 
-static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count)
+static void __n_tty_receive_buf(struct tty_struct *tty,
+				struct n_tty_data *ldata,
+				const unsigned char *cp, char *fp, int count)
 {
-	n_tty_receive_buf_common(tty, cp, fp, count, 0);
+	n_tty_receive_buf_common(tty, ldata, cp, fp, count, 0);
 }
 
-static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count)
+static int __n_tty_receive_buf2(struct tty_struct *tty,
+				struct n_tty_data *ldata,
+				const unsigned char *cp, char *fp, int count)
 {
-	return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+	return n_tty_receive_buf_common(tty, ldata, cp, fp, count, 1);
 }
 
 /**
@@ -1776,10 +1769,9 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
  *	Locking: Caller holds tty->termios_rwsem
  */
 
-static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
+static void __n_tty_set_termios(struct tty_struct *tty,
+				struct n_tty_data *ldata, struct ktermios *old)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & (ICANON | EXTPROC)) {
 		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
 		ldata->line_start = ldata->read_tail;
@@ -1852,7 +1844,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 	 */
 	if (!I_IXON(tty) && old && (old->c_iflag & IXON) && !tty->flow_stopped) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 
 	/* The termios change make the tty ready for I/O */
@@ -1869,7 +1861,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
  *	discipline change. The function will not be called while other
  *	ldisc methods are in progress.
  */
-
+static void put_n_tty(struct n_tty_data *ldata);
 static void n_tty_close(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
@@ -1877,8 +1869,8 @@ static void n_tty_close(struct tty_struct *tty)
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
 
-	vfree(ldata);
 	tty->disc_data = NULL;
+	put_n_tty(ldata);
 }
 
 /**
@@ -1890,7 +1882,7 @@ static void n_tty_close(struct tty_struct *tty)
  *	other events will occur in parallel. No further open will occur
  *	until a close.
  */
-
+static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old);
 static int n_tty_open(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata;
@@ -1900,6 +1892,7 @@ static int n_tty_open(struct tty_struct *tty)
 	if (!ldata)
 		return -ENOMEM;
 
+	refcount_set(&ldata->users, 1);
 	ldata->overrun_time = jiffies;
 	mutex_init(&ldata->atomic_read_lock);
 	mutex_init(&ldata->output_lock);
@@ -1913,9 +1906,9 @@ static int n_tty_open(struct tty_struct *tty)
 	return 0;
 }
 
-static inline int input_available_p(struct tty_struct *tty, int poll)
+static inline int input_available_p(struct tty_struct *tty,
+				    struct n_tty_data *ldata, int poll)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
 
 	if (ldata->icanon && !L_EXTPROC(tty))
@@ -1944,12 +1937,10 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
  *		read_tail published
  */
 
-static int copy_from_read_buf(struct tty_struct *tty,
-				      unsigned char __user **b,
-				      size_t *nr)
+static int copy_from_read_buf(struct tty_struct *tty, struct n_tty_data *ldata,
+			      unsigned char __user **b, size_t *nr)
 
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int retval;
 	size_t n;
 	bool is_eof;
@@ -2000,10 +1991,9 @@ static int copy_from_read_buf(struct tty_struct *tty,
  */
 
 static int canon_copy_from_read_buf(struct tty_struct *tty,
-				    unsigned char __user **b,
-				    size_t *nr)
+				    struct n_tty_data *ldata,
+				    unsigned char __user **b, size_t *nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, size, more, c;
 	size_t eol;
 	size_t tail;
@@ -2043,7 +2033,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
 	n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu tail:%zu more:%zu\n",
 		    __func__, eol, found, n, c, tail, more);
 
-	ret = tty_copy_to_user(tty, *b, tail, n);
+	ret = tty_copy_to_user(tty, ldata, *b, tail, n);
 	if (ret)
 		return -EFAULT;
 	*b += n;
@@ -2113,10 +2103,10 @@ static int job_control(struct tty_struct *tty, struct file *file)
  *		publishes read_tail
  */
 
-static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
-			 unsigned char __user *buf, size_t nr)
+static ssize_t __n_tty_read(struct tty_struct *tty, struct n_tty_data *ldata,
+			    struct file *file, unsigned char __user *buf,
+			    size_t nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	unsigned char __user *b = buf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	int c;
@@ -2178,11 +2168,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			break;
 		}
 
-		if (!input_available_p(tty, 0)) {
+		if (!input_available_p(tty, ldata, 0)) {
 			up_read(&tty->termios_rwsem);
 			tty_buffer_flush_work(tty->port);
 			down_read(&tty->termios_rwsem);
-			if (!input_available_p(tty, 0)) {
+			if (!input_available_p(tty, ldata, 0)) {
 				if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
 					retval = -EIO;
 					break;
@@ -2216,7 +2206,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 		}
 
 		if (ldata->icanon && !L_EXTPROC(tty)) {
-			retval = canon_copy_from_read_buf(tty, &b, &nr);
+			retval = canon_copy_from_read_buf(tty, ldata, &b, &nr);
 			if (retval)
 				break;
 		} else {
@@ -2232,15 +2222,15 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 				nr--;
 			}
 
-			uncopied = copy_from_read_buf(tty, &b, &nr);
-			uncopied += copy_from_read_buf(tty, &b, &nr);
+			uncopied = copy_from_read_buf(tty, ldata, &b, &nr);
+			uncopied += copy_from_read_buf(tty, ldata, &b, &nr);
 			if (uncopied) {
 				retval = -EFAULT;
 				break;
 			}
 		}
 
-		n_tty_check_unthrottle(tty);
+		n_tty_check_unthrottle(tty, ldata);
 
 		if (b - buf >= minimum)
 			break;
@@ -2248,7 +2238,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			timeout = time;
 	}
 	if (tail != ldata->read_tail)
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 	up_read(&tty->termios_rwsem);
 
 	remove_wait_queue(&tty->read_wait, &wait);
@@ -2282,8 +2272,9 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
  *		  lock themselves)
  */
 
-static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
-			   const unsigned char *buf, size_t nr)
+static ssize_t __n_tty_write(struct tty_struct *tty, struct n_tty_data *ldata,
+			     struct file *file, const unsigned char *buf,
+			     size_t nr)
 {
 	const unsigned char *b = buf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
@@ -2300,7 +2291,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 	down_read(&tty->termios_rwsem);
 
 	/* Write out any echoed characters that are still pending */
-	process_echoes(tty);
+	process_echoes(tty, ldata);
 
 	add_wait_queue(&tty->write_wait, &wait);
 	while (1) {
@@ -2314,7 +2305,8 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 		}
 		if (O_OPOST(tty)) {
 			while (nr > 0) {
-				ssize_t num = process_output_block(tty, b, nr);
+				ssize_t num = process_output_block(tty, ldata,
+								   b, nr);
 				if (num < 0) {
 					if (num == -EAGAIN)
 						break;
@@ -2326,15 +2318,13 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 				if (nr == 0)
 					break;
 				c = *b;
-				if (process_output(c, tty) < 0)
+				if (process_output(c, tty, ldata) < 0)
 					break;
 				b++; nr--;
 			}
 			if (tty->ops->flush_chars)
 				tty->ops->flush_chars(tty);
 		} else {
-			struct n_tty_data *ldata = tty->disc_data;
-
 			while (nr > 0) {
 				mutex_lock(&ldata->output_lock);
 				c = tty->ops->write(tty, b, nr);
@@ -2383,18 +2373,18 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
  *	Called without the kernel lock held - fine
  */
 
-static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
-							poll_table *wait)
+static __poll_t __n_tty_poll(struct tty_struct *tty, struct n_tty_data *ldata,
+			     struct file *file, poll_table *wait)
 {
 	__poll_t mask = 0;
 
 	poll_wait(file, &tty->read_wait, wait);
 	poll_wait(file, &tty->write_wait, wait);
-	if (input_available_p(tty, 1))
+	if (input_available_p(tty, ldata, 1))
 		mask |= EPOLLIN | EPOLLRDNORM;
 	else {
 		tty_buffer_flush_work(tty->port);
-		if (input_available_p(tty, 1))
+		if (input_available_p(tty, ldata, 1))
 			mask |= EPOLLIN | EPOLLRDNORM;
 	}
 	if (tty->packet && tty->link->ctrl_status)
@@ -2429,10 +2419,9 @@ static unsigned long inq_canon(struct n_tty_data *ldata)
 	return nr;
 }
 
-static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
-		       unsigned int cmd, unsigned long arg)
+static int __n_tty_ioctl(struct tty_struct *tty, struct n_tty_data *ldata,
+			 struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int retval;
 
 	switch (cmd) {
@@ -2451,6 +2440,130 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
 	}
 }
 
+static struct n_tty_data *tryget_n_tty(struct tty_struct *tty)
+{
+	struct n_tty_data *ldata;
+	bool ret;
+
+	rcu_read_lock();
+	ldata = tty->disc_data;
+	ret = ldata && refcount_inc_not_zero(&ldata->users);
+	rcu_read_unlock();
+	if (ret)
+		return ldata;
+	return NULL;
+}
+
+static void free_n_tty(struct rcu_head *head)
+{
+	struct n_tty_data *ldata = container_of(head, struct n_tty_data, rcu);
+
+	vfree(ldata);
+}
+
+static void put_n_tty(struct n_tty_data *ldata)
+{
+	if (refcount_dec_and_test(&ldata->users))
+		call_rcu(&ldata->rcu, free_n_tty);
+}
+
+static void n_tty_flush_buffer(struct tty_struct *tty)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_flush_buffer(tty, ldata);
+	put_n_tty(ldata);
+}
+
+static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
+			  unsigned char __user *buf, size_t nr)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	ssize_t retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_read(tty, ldata, file, buf, nr);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
+			   const unsigned char *buf, size_t nr)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	ssize_t retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_write(tty, ldata, file, buf, nr);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
+		       unsigned int cmd, unsigned long arg)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	int retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_ioctl(tty, ldata, file, cmd, arg);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_set_termios(tty, ldata, old);
+	put_n_tty(ldata);
+}
+
+static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
+			   poll_table *wait)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	__poll_t retval;
+
+	if (!ldata) /* What value is most appropriate for this case? */
+		return POLLERR|POLLHUP;
+	retval = __n_tty_poll(tty, ldata, file, wait);
+	put_n_tty(ldata);
+	return retval;
+
+}
+
+static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+			      char *fp, int count)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_receive_buf(tty, ldata, cp, fp, count);
+	put_n_tty(ldata);
+}
+
+static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
+			      char *fp, int count)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	int retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_receive_buf2(tty, ldata, cp, fp, count);
+	put_n_tty(ldata);
+	return retval;
+}
+
 static struct tty_ldisc_ops n_tty_ops = {
 	.magic           = TTY_LDISC_MAGIC,
 	.name            = "n_tty",
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-07-17 10:14     ` [PATCH] n_tty: Protect tty->disc_data using refcount Tetsuo Handa
@ 2018-07-24 15:22       ` Tetsuo Handa
  2018-07-25  8:06         ` Greg KH
  2018-08-29 13:53         ` Jiri Slaby
  0 siblings, 2 replies; 10+ messages in thread
From: Tetsuo Handa @ 2018-07-24 15:22 UTC (permalink / raw)
  To: gregkh, jslaby
  Cc: syzbot, linux-kernel, syzkaller-bugs, Alexander Viro, linux-fsdevel

From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 25 Jul 2018 00:15:18 +0900
Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.

syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.

Since we don't want to introduce new locking dependency, this patch
converts "struct n_tty_data *ldata = tty->disc_data;" in individual
function into a function argument which follows "struct tty *", and
holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
in order to ensure that memory which contains "struct n_tty_data" will
not be released while processing individual function.

[1] https://syzkaller.appspot.com/bug?id=1e850009fca0b64ce49dc16499bda4f7de0ab1a5

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/tty/n_tty.c | 511 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 314 insertions(+), 197 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4317422..0bb413b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -122,6 +122,10 @@ struct n_tty_data {
 
 	struct mutex atomic_read_lock;
 	struct mutex output_lock;
+
+	/* Race protection. */
+	refcount_t users;
+	struct rcu_head rcu;
 };
 
 #define MASK(x) ((x) & (N_TTY_BUF_SIZE - 1))
@@ -152,10 +156,9 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
 	return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
 }
 
-static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
-			    size_t tail, size_t n)
+static int tty_copy_to_user(struct tty_struct *tty, struct n_tty_data *ldata,
+			    void __user *to, size_t tail, size_t n)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t size = N_TTY_BUF_SIZE - tail;
 	const void *from = read_buf_addr(ldata, tail);
 	int uncopied;
@@ -186,10 +189,8 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
  *		holds non-exclusive termios_rwsem
  */
 
-static void n_tty_kick_worker(struct tty_struct *tty)
+static void n_tty_kick_worker(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	/* Did the input worker stop? Restart it */
 	if (unlikely(ldata->no_room)) {
 		ldata->no_room = 0;
@@ -206,9 +207,8 @@ static void n_tty_kick_worker(struct tty_struct *tty)
 	}
 }
 
-static ssize_t chars_in_buffer(struct tty_struct *tty)
+static ssize_t chars_in_buffer(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	ssize_t n = 0;
 
 	if (!ldata->icanon)
@@ -233,10 +233,9 @@ static void n_tty_write_wakeup(struct tty_struct *tty)
 	kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
 }
 
-static void n_tty_check_throttle(struct tty_struct *tty)
+static void n_tty_check_throttle(struct tty_struct *tty,
+				 struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	/*
 	 * Check the remaining room for the input canonicalization
 	 * mode.  We don't want to throttle the driver if we're in
@@ -257,12 +256,13 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 	__tty_set_flow_change(tty, 0);
 }
 
-static void n_tty_check_unthrottle(struct tty_struct *tty)
+static void n_tty_check_unthrottle(struct tty_struct *tty,
+				   struct n_tty_data *ldata)
 {
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
-		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+		if (chars_in_buffer(tty, ldata) > TTY_THRESHOLD_UNTHROTTLE)
 			return;
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 		tty_wakeup(tty->link);
 		return;
 	}
@@ -278,9 +278,9 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 	while (1) {
 		int unthrottled;
 		tty_set_flow_change(tty, TTY_UNTHROTTLE_SAFE);
-		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
+		if (chars_in_buffer(tty, ldata) > TTY_THRESHOLD_UNTHROTTLE)
 			break;
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 		unthrottled = tty_unthrottle_safe(tty);
 		if (!unthrottled)
 			break;
@@ -353,11 +353,12 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty)
  *	Locking: ctrl_lock, exclusive termios_rwsem
  */
 
-static void n_tty_flush_buffer(struct tty_struct *tty)
+static void __n_tty_flush_buffer(struct tty_struct *tty,
+				 struct n_tty_data *ldata)
 {
 	down_write(&tty->termios_rwsem);
-	reset_buffer_flags(tty->disc_data);
-	n_tty_kick_worker(tty);
+	reset_buffer_flags(ldata);
+	n_tty_kick_worker(tty, ldata);
 
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
@@ -413,9 +414,9 @@ static inline int is_continuation(unsigned char c, struct tty_struct *tty)
  *		 the column state and space left in the buffer
  */
 
-static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
+static int do_output_char(unsigned char c, struct tty_struct *tty,
+			  struct n_tty_data *ldata, int space)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	spaces;
 
 	if (!space)
@@ -488,15 +489,15 @@ static int do_output_char(unsigned char c, struct tty_struct *tty, int space)
  *		  tty layer write lock)
  */
 
-static int process_output(unsigned char c, struct tty_struct *tty)
+static int process_output(unsigned char c, struct tty_struct *tty,
+			  struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space, retval;
 
 	mutex_lock(&ldata->output_lock);
 
 	space = tty_write_room(tty);
-	retval = do_output_char(c, tty, space);
+	retval = do_output_char(c, tty, ldata, space);
 
 	mutex_unlock(&ldata->output_lock);
 	if (retval < 0)
@@ -525,9 +526,9 @@ static int process_output(unsigned char c, struct tty_struct *tty)
  */
 
 static ssize_t process_output_block(struct tty_struct *tty,
+				    struct n_tty_data *ldata,
 				    const unsigned char *buf, unsigned int nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space;
 	int	i;
 	const unsigned char *cp;
@@ -608,9 +609,8 @@ static ssize_t process_output_block(struct tty_struct *tty,
  *	Locking: callers must hold output_lock
  */
 
-static size_t __process_echoes(struct tty_struct *tty)
+static size_t __process_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int	space, old_space;
 	size_t tail;
 	unsigned char c;
@@ -721,7 +721,8 @@ static size_t __process_echoes(struct tty_struct *tty)
 				break;
 		} else {
 			if (O_OPOST(tty)) {
-				int retval = do_output_char(c, tty, space);
+				int retval = do_output_char(c, tty, ldata,
+							    space);
 				if (retval < 0)
 					break;
 				space -= retval;
@@ -754,9 +755,8 @@ static size_t __process_echoes(struct tty_struct *tty)
 	return old_space - space;
 }
 
-static void commit_echoes(struct tty_struct *tty)
+static void commit_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t nr, old, echoed;
 	size_t head;
 
@@ -776,16 +776,15 @@ static void commit_echoes(struct tty_struct *tty)
 	}
 
 	ldata->echo_commit = head;
-	echoed = __process_echoes(tty);
+	echoed = __process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 
 	if (echoed && tty->ops->flush_chars)
 		tty->ops->flush_chars(tty);
 }
 
-static void process_echoes(struct tty_struct *tty)
+static void process_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t echoed;
 
 	if (ldata->echo_mark == ldata->echo_tail)
@@ -793,7 +792,7 @@ static void process_echoes(struct tty_struct *tty)
 
 	mutex_lock(&ldata->output_lock);
 	ldata->echo_commit = ldata->echo_mark;
-	echoed = __process_echoes(tty);
+	echoed = __process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 
 	if (echoed && tty->ops->flush_chars)
@@ -801,17 +800,15 @@ static void process_echoes(struct tty_struct *tty)
 }
 
 /* NB: echo_mark and echo_head should be equivalent here */
-static void flush_echoes(struct tty_struct *tty)
+static void flush_echoes(struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if ((!L_ECHO(tty) && !L_ECHONL(tty)) ||
 	    ldata->echo_commit == ldata->echo_head)
 		return;
 
 	mutex_lock(&ldata->output_lock);
 	ldata->echo_commit = ldata->echo_head;
-	__process_echoes(tty);
+	__process_echoes(tty, ldata);
 	mutex_unlock(&ldata->output_lock);
 }
 
@@ -921,10 +918,9 @@ static void echo_char_raw(unsigned char c, struct n_tty_data *ldata)
  *	(where X is the letter representing the control char).
  */
 
-static void echo_char(unsigned char c, struct tty_struct *tty)
+static void echo_char(unsigned char c, struct tty_struct *tty,
+		      struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (c == ECHO_OP_START) {
 		add_echo_byte(ECHO_OP_START, ldata);
 		add_echo_byte(ECHO_OP_START, ldata);
@@ -961,9 +957,9 @@ static inline void finish_erasing(struct n_tty_data *ldata)
  *		caller holds non-exclusive termios_rwsem
  */
 
-static void eraser(unsigned char c, struct tty_struct *tty)
+static void eraser(unsigned char c, struct tty_struct *tty,
+		   struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	enum { ERASE, WERASE, KILL } kill_type;
 	size_t head;
 	size_t cnt;
@@ -985,7 +981,7 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 		if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
 			ldata->read_head = ldata->canon_head;
 			finish_erasing(ldata);
-			echo_char(KILL_CHAR(tty), tty);
+			echo_char(KILL_CHAR(tty), tty, ldata);
 			/* Add a newline if ECHOK is on and ECHOKE is off. */
 			if (L_ECHOK(tty))
 				echo_char_raw('\n', ldata);
@@ -1025,14 +1021,14 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 					ldata->erasing = 1;
 				}
 				/* if cnt > 1, output a multi-byte character */
-				echo_char(c, tty);
+				echo_char(c, tty, ldata);
 				while (--cnt > 0) {
 					head++;
 					echo_char_raw(read_buf(ldata, head), ldata);
 					echo_move_back_col(ldata);
 				}
 			} else if (kill_type == ERASE && !L_ECHOE(tty)) {
-				echo_char(ERASE_CHAR(tty), tty);
+				echo_char(ERASE_CHAR(tty), tty, ldata);
 			} else if (c == '\t') {
 				unsigned int num_chars = 0;
 				int after_tab = 0;
@@ -1103,10 +1099,8 @@ static void __isig(int sig, struct tty_struct *tty)
 	}
 }
 
-static void isig(int sig, struct tty_struct *tty)
+static void isig(int sig, struct tty_struct *tty, struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (L_NOFLSH(tty)) {
 		/* signal only */
 		__isig(sig, tty);
@@ -1127,7 +1121,7 @@ static void isig(int sig, struct tty_struct *tty)
 		tty_driver_flush_buffer(tty);
 
 		/* clear input buffer */
-		reset_buffer_flags(tty->disc_data);
+		reset_buffer_flags(ldata);
 
 		/* notify pty master of flush */
 		if (tty->link)
@@ -1151,14 +1145,13 @@ static void isig(int sig, struct tty_struct *tty)
  *	Note: may get exclusive termios_rwsem if flushing input buffer
  */
 
-static void n_tty_receive_break(struct tty_struct *tty)
+static void n_tty_receive_break(struct tty_struct *tty,
+				struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_IGNBRK(tty))
 		return;
 	if (I_BRKINT(tty)) {
-		isig(SIGINT, tty);
+		isig(SIGINT, tty, ldata);
 		return;
 	}
 	if (I_PARMRK(tty)) {
@@ -1181,10 +1174,9 @@ static void n_tty_receive_break(struct tty_struct *tty)
  *	private.
  */
 
-static void n_tty_receive_overrun(struct tty_struct *tty)
+static void n_tty_receive_overrun(struct tty_struct *tty,
+				  struct n_tty_data *ldata)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	ldata->num_overrun++;
 	if (time_after(jiffies, ldata->overrun_time + HZ) ||
 			time_after(ldata->overrun_time, jiffies)) {
@@ -1205,10 +1197,10 @@ static void n_tty_receive_overrun(struct tty_struct *tty)
  *	n_tty_receive_buf()/producer path:
  *		caller holds non-exclusive termios_rwsem
  */
-static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_parity_error(struct tty_struct *tty,
+				       struct n_tty_data *ldata,
+				       unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_INPCK(tty)) {
 		if (I_IGNPAR(tty))
 			return;
@@ -1223,16 +1215,17 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
+n_tty_receive_signal_char(struct tty_struct *tty, struct n_tty_data *ldata,
+			  int signal, unsigned char c)
 {
-	isig(signal, tty);
+	isig(signal, tty, ldata);
 	if (I_IXON(tty))
 		start_tty(tty);
 	if (L_ECHO(tty)) {
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	} else
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	return;
 }
 
@@ -1253,14 +1246,13 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
  */
 
 static int
-n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_special(struct tty_struct *tty, struct n_tty_data *ldata,
+			   unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (I_IXON(tty)) {
 		if (c == START_CHAR(tty)) {
 			start_tty(tty);
-			process_echoes(tty);
+			process_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == STOP_CHAR(tty)) {
@@ -1271,20 +1263,20 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 
 	if (L_ISIG(tty)) {
 		if (c == INTR_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGINT, c);
+			n_tty_receive_signal_char(tty, ldata, SIGINT, c);
 			return 0;
 		} else if (c == QUIT_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGQUIT, c);
+			n_tty_receive_signal_char(tty, ldata, SIGQUIT, c);
 			return 0;
 		} else if (c == SUSP_CHAR(tty)) {
-			n_tty_receive_signal_char(tty, SIGTSTP, c);
+			n_tty_receive_signal_char(tty, ldata, SIGTSTP, c);
 			return 0;
 		}
 	}
 
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 
 	if (c == '\r') {
@@ -1298,8 +1290,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 	if (ldata->icanon) {
 		if (c == ERASE_CHAR(tty) || c == KILL_CHAR(tty) ||
 		    (c == WERASE_CHAR(tty) && L_IEXTEN(tty))) {
-			eraser(c, tty);
-			commit_echoes(tty);
+			eraser(c, tty, ldata);
+			commit_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == LNEXT_CHAR(tty) && L_IEXTEN(tty)) {
@@ -1309,7 +1301,7 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 				if (L_ECHOCTL(tty)) {
 					echo_char_raw('^', ldata);
 					echo_char_raw('\b', ldata);
-					commit_echoes(tty);
+					commit_echoes(tty, ldata);
 				}
 			}
 			return 1;
@@ -1318,19 +1310,19 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 			size_t tail = ldata->canon_head;
 
 			finish_erasing(ldata);
-			echo_char(c, tty);
+			echo_char(c, tty, ldata);
 			echo_char_raw('\n', ldata);
 			while (MASK(tail) != MASK(ldata->read_head)) {
-				echo_char(read_buf(ldata, tail), tty);
+				echo_char(read_buf(ldata, tail), tty, ldata);
 				tail++;
 			}
-			commit_echoes(tty);
+			commit_echoes(tty, ldata);
 			return 0;
 		}
 		if (c == '\n') {
 			if (L_ECHO(tty) || L_ECHONL(tty)) {
 				echo_char_raw('\n', ldata);
-				commit_echoes(tty);
+				commit_echoes(tty, ldata);
 			}
 			goto handle_newline;
 		}
@@ -1347,8 +1339,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 				/* Record the column of first canon char. */
 				if (ldata->canon_head == ldata->read_head)
 					echo_set_canon_col(ldata);
-				echo_char(c, tty);
-				commit_echoes(tty);
+				echo_char(c, tty, ldata);
+				commit_echoes(tty, ldata);
 			}
 			/*
 			 * XXX does PARMRK doubling happen for
@@ -1375,9 +1367,9 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 			/* Record the column of first canon char. */
 			if (ldata->canon_head == ldata->read_head)
 				echo_set_canon_col(ldata);
-			echo_char(c, tty);
+			echo_char(c, tty, ldata);
 		}
-		commit_echoes(tty);
+		commit_echoes(tty, ldata);
 	}
 
 	/* PARMRK doubling check */
@@ -1389,21 +1381,20 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 }
 
 static inline void
-n_tty_receive_char_inline(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_inline(struct tty_struct *tty, struct n_tty_data *ldata,
+			  unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 	if (L_ECHO(tty)) {
 		finish_erasing(ldata);
 		/* Record the column of first canon char. */
 		if (ldata->canon_head == ldata->read_head)
 			echo_set_canon_col(ldata);
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	}
 	/* PARMRK doubling check */
 	if (c == (unsigned char) '\377' && I_PARMRK(tty))
@@ -1411,32 +1402,34 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c)
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char(struct tty_struct *tty, struct n_tty_data *ldata,
+			       unsigned char c)
 {
-	n_tty_receive_char_inline(tty, c);
+	n_tty_receive_char_inline(tty, ldata, c);
 }
 
 static inline void
-n_tty_receive_char_fast(struct tty_struct *tty, unsigned char c)
+n_tty_receive_char_fast(struct tty_struct *tty, struct n_tty_data *ldata,
+			unsigned char c)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty)) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 	if (L_ECHO(tty)) {
 		finish_erasing(ldata);
 		/* Record the column of first canon char. */
 		if (ldata->canon_head == ldata->read_head)
 			echo_set_canon_col(ldata);
-		echo_char(c, tty);
-		commit_echoes(tty);
+		echo_char(c, tty, ldata);
+		commit_echoes(tty, ldata);
 	}
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_closing(struct tty_struct *tty,
+				       struct n_tty_data *ldata,
+				       unsigned char c)
 {
 	if (I_ISTRIP(tty))
 		c &= 0x7f;
@@ -1451,24 +1444,25 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 			  c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
 			  c != SUSP_CHAR(tty))) {
 			start_tty(tty);
-			process_echoes(tty);
+			process_echoes(tty, ldata);
 		}
 	}
 }
 
 static void
-n_tty_receive_char_flagged(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_flagged(struct tty_struct *tty, struct n_tty_data *ldata,
+			   unsigned char c, char flag)
 {
 	switch (flag) {
 	case TTY_BREAK:
-		n_tty_receive_break(tty);
+		n_tty_receive_break(tty, ldata);
 		break;
 	case TTY_PARITY:
 	case TTY_FRAME:
-		n_tty_receive_parity_error(tty, c);
+		n_tty_receive_parity_error(tty, ldata, c);
 		break;
 	case TTY_OVERRUN:
-		n_tty_receive_overrun(tty);
+		n_tty_receive_overrun(tty, ldata);
 		break;
 	default:
 		tty_err(tty, "unknown flag %d\n", flag);
@@ -1477,26 +1471,24 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
+n_tty_receive_char_lnext(struct tty_struct *tty, struct n_tty_data *ldata,
+			 unsigned char c, char flag)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	ldata->lnext = 0;
 	if (likely(flag == TTY_NORMAL)) {
 		if (I_ISTRIP(tty))
 			c &= 0x7f;
 		if (I_IUCLC(tty) && L_IEXTEN(tty))
 			c = tolower(c);
-		n_tty_receive_char(tty, c);
+		n_tty_receive_char(tty, ldata, c);
 	} else
-		n_tty_receive_char_flagged(tty, c, flag);
+		n_tty_receive_char_flagged(tty, ldata, c, flag);
 }
 
 static void
-n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
-			   char *fp, int count)
+n_tty_receive_buf_real_raw(struct tty_struct *tty, struct n_tty_data *ldata,
+			   const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, head;
 
 	head = ldata->read_head & (N_TTY_BUF_SIZE - 1);
@@ -1513,10 +1505,9 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 }
 
 static void
-n_tty_receive_buf_raw(struct tty_struct *tty, const unsigned char *cp,
-		      char *fp, int count)
+n_tty_receive_buf_raw(struct tty_struct *tty, struct n_tty_data *ldata,
+		      const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1525,13 +1516,13 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 		if (likely(flag == TTY_NORMAL))
 			put_tty_queue(*cp++, ldata);
 		else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
 static void
-n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+n_tty_receive_buf_closing(struct tty_struct *tty, struct n_tty_data *ldata,
+			  const unsigned char *cp, char *fp, int count)
 {
 	char flag = TTY_NORMAL;
 
@@ -1539,15 +1530,14 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 		if (fp)
 			flag = *fp++;
 		if (likely(flag == TTY_NORMAL))
-			n_tty_receive_char_closing(tty, *cp++);
+			n_tty_receive_char_closing(tty, ldata, *cp++);
 	}
 }
 
 static void
-n_tty_receive_buf_standard(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+n_tty_receive_buf_standard(struct tty_struct *tty, struct n_tty_data *ldata,
+			   const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1565,23 +1555,24 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 				continue;
 			}
 			if (!test_bit(c, ldata->char_map))
-				n_tty_receive_char_inline(tty, c);
-			else if (n_tty_receive_char_special(tty, c) && count) {
+				n_tty_receive_char_inline(tty, ldata, c);
+			else if (n_tty_receive_char_special(tty, ldata, c) &&
+				 count) {
 				if (fp)
 					flag = *fp++;
-				n_tty_receive_char_lnext(tty, *cp++, flag);
+				n_tty_receive_char_lnext(tty, ldata, *cp++,
+							 flag);
 				count--;
 			}
 		} else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
 static void
-n_tty_receive_buf_fast(struct tty_struct *tty, const unsigned char *cp,
-		       char *fp, int count)
+n_tty_receive_buf_fast(struct tty_struct *tty, struct n_tty_data *ldata,
+		       const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1591,46 +1582,47 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 			unsigned char c = *cp++;
 
 			if (!test_bit(c, ldata->char_map))
-				n_tty_receive_char_fast(tty, c);
-			else if (n_tty_receive_char_special(tty, c) && count) {
+				n_tty_receive_char_fast(tty, ldata, c);
+			else if (n_tty_receive_char_special(tty, ldata, c) &&
+				 count) {
 				if (fp)
 					flag = *fp++;
-				n_tty_receive_char_lnext(tty, *cp++, flag);
+				n_tty_receive_char_lnext(tty, ldata, *cp++,
+							 flag);
 				count--;
 			}
 		} else
-			n_tty_receive_char_flagged(tty, *cp++, flag);
+			n_tty_receive_char_flagged(tty, ldata, *cp++, flag);
 	}
 }
 
-static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			  char *fp, int count)
+static void __receive_buf(struct tty_struct *tty, struct n_tty_data *ldata,
+			  const unsigned char *cp, char *fp, int count)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
 
 	if (ldata->real_raw)
-		n_tty_receive_buf_real_raw(tty, cp, fp, count);
+		n_tty_receive_buf_real_raw(tty, ldata, cp, fp, count);
 	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
-		n_tty_receive_buf_raw(tty, cp, fp, count);
+		n_tty_receive_buf_raw(tty, ldata, cp, fp, count);
 	else if (tty->closing && !L_EXTPROC(tty))
-		n_tty_receive_buf_closing(tty, cp, fp, count);
+		n_tty_receive_buf_closing(tty, ldata, cp, fp, count);
 	else {
 		if (ldata->lnext) {
 			char flag = TTY_NORMAL;
 
 			if (fp)
 				flag = *fp++;
-			n_tty_receive_char_lnext(tty, *cp++, flag);
+			n_tty_receive_char_lnext(tty, ldata, *cp++, flag);
 			count--;
 		}
 
 		if (!preops && !I_PARMRK(tty))
-			n_tty_receive_buf_fast(tty, cp, fp, count);
+			n_tty_receive_buf_fast(tty, ldata, cp, fp, count);
 		else
-			n_tty_receive_buf_standard(tty, cp, fp, count);
+			n_tty_receive_buf_standard(tty, ldata, cp, fp, count);
 
-		flush_echoes(tty);
+		flush_echoes(tty, ldata);
 		if (tty->ops->flush_chars)
 			tty->ops->flush_chars(tty);
 	}
@@ -1681,10 +1673,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
  *		publishes commit_head or canon_head
  */
 static int
-n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
-			 char *fp, int count, int flow)
+n_tty_receive_buf_common(struct tty_struct *tty, struct n_tty_data *ldata,
+			 const unsigned char *cp, char *fp, int count, int flow)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int room, n, rcvd = 0, overflow;
 
 	down_read(&tty->termios_rwsem);
@@ -1724,7 +1715,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 		/* ignore parity errors if handling overflow */
 		if (!overflow || !fp || *fp != TTY_PARITY)
-			__receive_buf(tty, cp, fp, n);
+			__receive_buf(tty, ldata, cp, fp, n);
 
 		cp += n;
 		if (fp)
@@ -1743,23 +1734,25 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			__tty_set_flow_change(tty, 0);
 		}
 	} else
-		n_tty_check_throttle(tty);
+		n_tty_check_throttle(tty, ldata);
 
 	up_read(&tty->termios_rwsem);
 
 	return rcvd;
 }
 
-static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count)
+static void __n_tty_receive_buf(struct tty_struct *tty,
+				struct n_tty_data *ldata,
+				const unsigned char *cp, char *fp, int count)
 {
-	n_tty_receive_buf_common(tty, cp, fp, count, 0);
+	n_tty_receive_buf_common(tty, ldata, cp, fp, count, 0);
 }
 
-static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
-			      char *fp, int count)
+static int __n_tty_receive_buf2(struct tty_struct *tty,
+				struct n_tty_data *ldata,
+				const unsigned char *cp, char *fp, int count)
 {
-	return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+	return n_tty_receive_buf_common(tty, ldata, cp, fp, count, 1);
 }
 
 /**
@@ -1776,10 +1769,9 @@ static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
  *	Locking: Caller holds tty->termios_rwsem
  */
 
-static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
+static void __n_tty_set_termios(struct tty_struct *tty,
+				struct n_tty_data *ldata, struct ktermios *old)
 {
-	struct n_tty_data *ldata = tty->disc_data;
-
 	if (!old || (old->c_lflag ^ tty->termios.c_lflag) & (ICANON | EXTPROC)) {
 		bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
 		ldata->line_start = ldata->read_tail;
@@ -1852,7 +1844,7 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 	 */
 	if (!I_IXON(tty) && old && (old->c_iflag & IXON) && !tty->flow_stopped) {
 		start_tty(tty);
-		process_echoes(tty);
+		process_echoes(tty, ldata);
 	}
 
 	/* The termios change make the tty ready for I/O */
@@ -1869,16 +1861,20 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
  *	discipline change. The function will not be called while other
  *	ldisc methods are in progress.
  */
-
+static void put_n_tty(struct n_tty_data *ldata);
 static void n_tty_close(struct tty_struct *tty)
 {
-	struct n_tty_data *ldata = tty->disc_data;
+	struct n_tty_data *ldata = xchg(&tty->disc_data, NULL);
 
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
 
-	vfree(ldata);
-	tty->disc_data = NULL;
+	/*
+	 * The xchg() above and this NULL test are rather paranoid checks.
+	 * Caller should not call close() twice.
+	 */
+	if (ldata)
+		put_n_tty(ldata);
 }
 
 /**
@@ -1890,7 +1886,7 @@ static void n_tty_close(struct tty_struct *tty)
  *	other events will occur in parallel. No further open will occur
  *	until a close.
  */
-
+static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old);
 static int n_tty_open(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata;
@@ -1900,6 +1896,7 @@ static int n_tty_open(struct tty_struct *tty)
 	if (!ldata)
 		return -ENOMEM;
 
+	refcount_set(&ldata->users, 1);
 	ldata->overrun_time = jiffies;
 	mutex_init(&ldata->atomic_read_lock);
 	mutex_init(&ldata->output_lock);
@@ -1913,9 +1910,9 @@ static int n_tty_open(struct tty_struct *tty)
 	return 0;
 }
 
-static inline int input_available_p(struct tty_struct *tty, int poll)
+static inline int input_available_p(struct tty_struct *tty,
+				    struct n_tty_data *ldata, int poll)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
 
 	if (ldata->icanon && !L_EXTPROC(tty))
@@ -1944,12 +1941,10 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
  *		read_tail published
  */
 
-static int copy_from_read_buf(struct tty_struct *tty,
-				      unsigned char __user **b,
-				      size_t *nr)
+static int copy_from_read_buf(struct tty_struct *tty, struct n_tty_data *ldata,
+			      unsigned char __user **b, size_t *nr)
 
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int retval;
 	size_t n;
 	bool is_eof;
@@ -2000,10 +1995,9 @@ static int copy_from_read_buf(struct tty_struct *tty,
  */
 
 static int canon_copy_from_read_buf(struct tty_struct *tty,
-				    unsigned char __user **b,
-				    size_t *nr)
+				    struct n_tty_data *ldata,
+				    unsigned char __user **b, size_t *nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	size_t n, size, more, c;
 	size_t eol;
 	size_t tail;
@@ -2043,7 +2037,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
 	n_tty_trace("%s: eol:%zu found:%d n:%zu c:%zu tail:%zu more:%zu\n",
 		    __func__, eol, found, n, c, tail, more);
 
-	ret = tty_copy_to_user(tty, *b, tail, n);
+	ret = tty_copy_to_user(tty, ldata, *b, tail, n);
 	if (ret)
 		return -EFAULT;
 	*b += n;
@@ -2113,10 +2107,10 @@ static int job_control(struct tty_struct *tty, struct file *file)
  *		publishes read_tail
  */
 
-static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
-			 unsigned char __user *buf, size_t nr)
+static ssize_t __n_tty_read(struct tty_struct *tty, struct n_tty_data *ldata,
+			    struct file *file, unsigned char __user *buf,
+			    size_t nr)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	unsigned char __user *b = buf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	int c;
@@ -2178,11 +2172,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			break;
 		}
 
-		if (!input_available_p(tty, 0)) {
+		if (!input_available_p(tty, ldata, 0)) {
 			up_read(&tty->termios_rwsem);
 			tty_buffer_flush_work(tty->port);
 			down_read(&tty->termios_rwsem);
-			if (!input_available_p(tty, 0)) {
+			if (!input_available_p(tty, ldata, 0)) {
 				if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
 					retval = -EIO;
 					break;
@@ -2216,7 +2210,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 		}
 
 		if (ldata->icanon && !L_EXTPROC(tty)) {
-			retval = canon_copy_from_read_buf(tty, &b, &nr);
+			retval = canon_copy_from_read_buf(tty, ldata, &b, &nr);
 			if (retval)
 				break;
 		} else {
@@ -2232,15 +2226,15 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 				nr--;
 			}
 
-			uncopied = copy_from_read_buf(tty, &b, &nr);
-			uncopied += copy_from_read_buf(tty, &b, &nr);
+			uncopied = copy_from_read_buf(tty, ldata, &b, &nr);
+			uncopied += copy_from_read_buf(tty, ldata, &b, &nr);
 			if (uncopied) {
 				retval = -EFAULT;
 				break;
 			}
 		}
 
-		n_tty_check_unthrottle(tty);
+		n_tty_check_unthrottle(tty, ldata);
 
 		if (b - buf >= minimum)
 			break;
@@ -2248,7 +2242,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			timeout = time;
 	}
 	if (tail != ldata->read_tail)
-		n_tty_kick_worker(tty);
+		n_tty_kick_worker(tty, ldata);
 	up_read(&tty->termios_rwsem);
 
 	remove_wait_queue(&tty->read_wait, &wait);
@@ -2282,8 +2276,9 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
  *		  lock themselves)
  */
 
-static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
-			   const unsigned char *buf, size_t nr)
+static ssize_t __n_tty_write(struct tty_struct *tty, struct n_tty_data *ldata,
+			     struct file *file, const unsigned char *buf,
+			     size_t nr)
 {
 	const unsigned char *b = buf;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
@@ -2300,7 +2295,7 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 	down_read(&tty->termios_rwsem);
 
 	/* Write out any echoed characters that are still pending */
-	process_echoes(tty);
+	process_echoes(tty, ldata);
 
 	add_wait_queue(&tty->write_wait, &wait);
 	while (1) {
@@ -2314,7 +2309,8 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 		}
 		if (O_OPOST(tty)) {
 			while (nr > 0) {
-				ssize_t num = process_output_block(tty, b, nr);
+				ssize_t num = process_output_block(tty, ldata,
+								   b, nr);
 				if (num < 0) {
 					if (num == -EAGAIN)
 						break;
@@ -2326,15 +2322,13 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
 				if (nr == 0)
 					break;
 				c = *b;
-				if (process_output(c, tty) < 0)
+				if (process_output(c, tty, ldata) < 0)
 					break;
 				b++; nr--;
 			}
 			if (tty->ops->flush_chars)
 				tty->ops->flush_chars(tty);
 		} else {
-			struct n_tty_data *ldata = tty->disc_data;
-
 			while (nr > 0) {
 				mutex_lock(&ldata->output_lock);
 				c = tty->ops->write(tty, b, nr);
@@ -2383,18 +2377,18 @@ static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
  *	Called without the kernel lock held - fine
  */
 
-static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
-							poll_table *wait)
+static __poll_t __n_tty_poll(struct tty_struct *tty, struct n_tty_data *ldata,
+			     struct file *file, poll_table *wait)
 {
 	__poll_t mask = 0;
 
 	poll_wait(file, &tty->read_wait, wait);
 	poll_wait(file, &tty->write_wait, wait);
-	if (input_available_p(tty, 1))
+	if (input_available_p(tty, ldata, 1))
 		mask |= EPOLLIN | EPOLLRDNORM;
 	else {
 		tty_buffer_flush_work(tty->port);
-		if (input_available_p(tty, 1))
+		if (input_available_p(tty, ldata, 1))
 			mask |= EPOLLIN | EPOLLRDNORM;
 	}
 	if (tty->packet && tty->link->ctrl_status)
@@ -2429,10 +2423,9 @@ static unsigned long inq_canon(struct n_tty_data *ldata)
 	return nr;
 }
 
-static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
-		       unsigned int cmd, unsigned long arg)
+static int __n_tty_ioctl(struct tty_struct *tty, struct n_tty_data *ldata,
+			 struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct n_tty_data *ldata = tty->disc_data;
 	int retval;
 
 	switch (cmd) {
@@ -2451,6 +2444,130 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
 	}
 }
 
+static struct n_tty_data *tryget_n_tty(struct tty_struct *tty)
+{
+	struct n_tty_data *ldata;
+	bool ret;
+
+	rcu_read_lock();
+	ldata = tty->disc_data;
+	ret = ldata && refcount_inc_not_zero(&ldata->users);
+	rcu_read_unlock();
+	if (ret)
+		return ldata;
+	return NULL;
+}
+
+static void free_n_tty(struct rcu_head *head)
+{
+	struct n_tty_data *ldata = container_of(head, struct n_tty_data, rcu);
+
+	vfree(ldata);
+}
+
+static void put_n_tty(struct n_tty_data *ldata)
+{
+	if (refcount_dec_and_test(&ldata->users))
+		call_rcu(&ldata->rcu, free_n_tty);
+}
+
+static void n_tty_flush_buffer(struct tty_struct *tty)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_flush_buffer(tty, ldata);
+	put_n_tty(ldata);
+}
+
+static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
+			  unsigned char __user *buf, size_t nr)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	ssize_t retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_read(tty, ldata, file, buf, nr);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static ssize_t n_tty_write(struct tty_struct *tty, struct file *file,
+			   const unsigned char *buf, size_t nr)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	ssize_t retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_write(tty, ldata, file, buf, nr);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
+		       unsigned int cmd, unsigned long arg)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	int retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_ioctl(tty, ldata, file, cmd, arg);
+	put_n_tty(ldata);
+	return retval;
+}
+
+static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_set_termios(tty, ldata, old);
+	put_n_tty(ldata);
+}
+
+static __poll_t n_tty_poll(struct tty_struct *tty, struct file *file,
+			   poll_table *wait)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	__poll_t retval;
+
+	if (!ldata)
+		return POLLERR|POLLHUP;
+	retval = __n_tty_poll(tty, ldata, file, wait);
+	put_n_tty(ldata);
+	return retval;
+
+}
+
+static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+			      char *fp, int count)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+
+	if (!ldata)
+		return;
+	__n_tty_receive_buf(tty, ldata, cp, fp, count);
+	put_n_tty(ldata);
+}
+
+static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
+			      char *fp, int count)
+{
+	struct n_tty_data *ldata = tryget_n_tty(tty);
+	int retval;
+
+	if (!ldata)
+		return -EIO;
+	retval = __n_tty_receive_buf2(tty, ldata, cp, fp, count);
+	put_n_tty(ldata);
+	return retval;
+}
+
 static struct tty_ldisc_ops n_tty_ops = {
 	.magic           = TTY_LDISC_MAGIC,
 	.name            = "n_tty",
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-07-24 15:22       ` [PATCH v2] " Tetsuo Handa
@ 2018-07-25  8:06         ` Greg KH
  2018-07-25 12:35           ` Tetsuo Handa
  2018-08-29 13:53         ` Jiri Slaby
  1 sibling, 1 reply; 10+ messages in thread
From: Greg KH @ 2018-07-25  8:06 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: jslaby, syzbot, linux-kernel, syzkaller-bugs, Alexander Viro,
	linux-fsdevel

On Wed, Jul 25, 2018 at 12:22:16AM +0900, Tetsuo Handa wrote:
> >From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Wed, 25 Jul 2018 00:15:18 +0900
> Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
> 
> syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
> This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
> 
> Since we don't want to introduce new locking dependency, this patch
> converts "struct n_tty_data *ldata = tty->disc_data;" in individual
> function into a function argument which follows "struct tty *", and
> holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
> in order to ensure that memory which contains "struct n_tty_data" will
> not be released while processing individual function.
> 
> [1] https://syzkaller.appspot.com/bug?id=1e850009fca0b64ce49dc16499bda4f7de0ab1a5
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  drivers/tty/n_tty.c | 511 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 314 insertions(+), 197 deletions(-)

What changed from v1?  I haven't had the chance to review your first
patch, sorry, it's still in my queue.  I was hoping that someone else
would help out with that as well :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-07-25  8:06         ` Greg KH
@ 2018-07-25 12:35           ` Tetsuo Handa
  0 siblings, 0 replies; 10+ messages in thread
From: Tetsuo Handa @ 2018-07-25 12:35 UTC (permalink / raw)
  To: Greg KH
  Cc: jslaby, syzbot, linux-kernel, syzkaller-bugs, Alexander Viro,
	linux-fsdevel

On 2018/07/25 17:06, Greg KH wrote:
> On Wed, Jul 25, 2018 at 12:22:16AM +0900, Tetsuo Handa wrote:
>> >From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Date: Wed, 25 Jul 2018 00:15:18 +0900
>> Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
>>
>> syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
>> This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
>>
>> Since we don't want to introduce new locking dependency, this patch
>> converts "struct n_tty_data *ldata = tty->disc_data;" in individual
>> function into a function argument which follows "struct tty *", and
>> holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
>> in order to ensure that memory which contains "struct n_tty_data" will
>> not be released while processing individual function.
>>
>> [1] https://syzkaller.appspot.com/bug?id=1e850009fca0b64ce49dc16499bda4f7de0ab1a5
>>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> ---
>>  drivers/tty/n_tty.c | 511 ++++++++++++++++++++++++++++++++--------------------
>>  1 file changed, 314 insertions(+), 197 deletions(-)
> 
> What changed from v1?  I haven't had the chance to review your first
> patch, sorry, it's still in my queue.  I was hoping that someone else
> would help out with that as well :)
> 

Just added a check in case I overlooked a subtle race condition that
n_tty_close() is _somehow_ called twice.

  25,26c20,21
  <  1 file changed, 308 insertions(+), 195 deletions(-)
  >  1 file changed, 314 insertions(+), 197 deletions(-)
  872,873c867,869
  <       struct n_tty_data *ldata = tty->disc_data;
  > -     struct n_tty_data *ldata = tty->disc_data;
  > +     struct n_tty_data *ldata = xchg(&tty->disc_data, NULL);
  >
  878,879c874,880
  <       tty->disc_data = NULL;
  < +     put_n_tty(ldata);
  > -     tty->disc_data = NULL;
  > +     /*
  > +      * The xchg() above and this NULL test are rather paranoid checks.
  > +      * Caller should not allow calling close() twice.
  > +      */
  > +     if (ldata)
  > +             put_n_tty(ldata);
  1194c1195
  < +     if (!ldata) /* What value is most appropriate for this case? */
  > +     if (!ldata)

If n_tty_close() is _somehow_ called twice, v1 patch will trigger NULL
pointer dereference. If you are sure that n_tty_close() is never called
twice, you can ignore v2.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-07-24 15:22       ` [PATCH v2] " Tetsuo Handa
  2018-07-25  8:06         ` Greg KH
@ 2018-08-29 13:53         ` Jiri Slaby
  2018-08-29 14:48           ` Jiri Slaby
  2018-08-29 15:02           ` Tetsuo Handa
  1 sibling, 2 replies; 10+ messages in thread
From: Jiri Slaby @ 2018-08-29 13:53 UTC (permalink / raw)
  To: Tetsuo Handa, gregkh, jslaby
  Cc: syzbot, linux-kernel, syzkaller-bugs, Alexander Viro, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On 07/24/2018, 05:22 PM, Tetsuo Handa wrote:
> From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Wed, 25 Jul 2018 00:15:18 +0900
> Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
> 
> syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
> This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
> 
> Since we don't want to introduce new locking dependency, this patch
> converts "struct n_tty_data *ldata = tty->disc_data;" in individual
> function into a function argument which follows "struct tty *", and
> holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
> in order to ensure that memory which contains "struct n_tty_data" will
> not be released while processing individual function.

This does not look correct and is way too complicated. ioctls should not
be called while changing/killing/hanging/whatever a ldisc. But there is
one missing lock in tty_reopen.

So does the attached patch helps instead?

thanks,
-- 
js
suse labs

[-- Attachment #2: 0001-tty-fix-NULL-ptr-dereference.patch --]
[-- Type: text/x-patch, Size: 1109 bytes --]

From a41b427c77d920cd151473361788deb80e576f6c Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Wed, 29 Aug 2018 15:49:51 +0200
Subject: [PATCH] tty: fix NULL ptr dereference

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/tty_io.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f89939922050..160f320e0d2c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1255,6 +1255,7 @@ static void tty_driver_remove_tty(struct tty_driver *driver, struct tty_struct *
 static int tty_reopen(struct tty_struct *tty)
 {
 	struct tty_driver *driver = tty->driver;
+	int ret = 0;
 
 	if (driver->type == TTY_DRIVER_TYPE_PTY &&
 	    driver->subtype == PTY_TYPE_MASTER)
@@ -1268,10 +1269,12 @@ static int tty_reopen(struct tty_struct *tty)
 
 	tty->count++;
 
+	tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT);
 	if (!tty->ldisc)
-		return tty_ldisc_reinit(tty, tty->termios.c_line);
+		ret = tty_ldisc_reinit(tty, tty->termios.c_line);
+	tty_ldisc_unlock(tty);
 
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-08-29 13:53         ` Jiri Slaby
@ 2018-08-29 14:48           ` Jiri Slaby
  2018-08-29 15:02           ` Tetsuo Handa
  1 sibling, 0 replies; 10+ messages in thread
From: Jiri Slaby @ 2018-08-29 14:48 UTC (permalink / raw)
  To: Tetsuo Handa, gregkh
  Cc: syzkaller-bugs, syzbot, linux-fsdevel, linux-kernel, Alexander Viro

On 08/29/2018, 03:53 PM, Jiri Slaby wrote:
> On 07/24/2018, 05:22 PM, Tetsuo Handa wrote:
>> From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Date: Wed, 25 Jul 2018 00:15:18 +0900
>> Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
>>
>> syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
>> This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
>>
>> Since we don't want to introduce new locking dependency, this patch
>> converts "struct n_tty_data *ldata = tty->disc_data;" in individual
>> function into a function argument which follows "struct tty *", and
>> holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
>> in order to ensure that memory which contains "struct n_tty_data" will
>> not be released while processing individual function.
> 
> This does not look correct and is way too complicated. ioctls should not
> be called while changing/killing/hanging/whatever a ldisc. But there is
> one missing lock in tty_reopen.
> 
> So does the attached patch helps instead?

Which is btw in fact semantically the same as Dmitry's patch:
https://lore.kernel.org/lkml/20180829022353.23568-3-dima@arista.com/

> thanks,-- 
js
suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
  2018-08-29 13:53         ` Jiri Slaby
  2018-08-29 14:48           ` Jiri Slaby
@ 2018-08-29 15:02           ` Tetsuo Handa
  1 sibling, 0 replies; 10+ messages in thread
From: Tetsuo Handa @ 2018-08-29 15:02 UTC (permalink / raw)
  To: Jiri Slaby, gregkh, jslaby
  Cc: syzbot, linux-kernel, syzkaller-bugs, Alexander Viro, linux-fsdevel

On 2018/08/29 22:53, Jiri Slaby wrote:
> On 07/24/2018, 05:22 PM, Tetsuo Handa wrote:
>> From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
>> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> Date: Wed, 25 Jul 2018 00:15:18 +0900
>> Subject: [PATCH v2] n_tty: Protect tty->disc_data using refcount.
>>
>> syzbot is reporting NULL pointer dereference at n_tty_set_termios() [1].
>> This is because ioctl(TIOCVHANGUP) versus ioctl(TCSETS) can race.
>>
>> Since we don't want to introduce new locking dependency, this patch
>> converts "struct n_tty_data *ldata = tty->disc_data;" in individual
>> function into a function argument which follows "struct tty *", and
>> holds tty->disc_data at each "struct tty_ldisc_ops" hook using refcount
>> in order to ensure that memory which contains "struct n_tty_data" will
>> not be released while processing individual function.
> 
> This does not look correct and is way too complicated. ioctls should not
> be called while changing/killing/hanging/whatever a ldisc. But there is
> one missing lock in tty_reopen.
> 
> So does the attached patch helps instead?
> 
> thanks,
> 

That patch seems to help avoiding crashes. (You can use #syz test: command.)
But I think you need to check tty_ldisc_lock() return value...

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-08-29 15:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-01 17:01 KASAN: user-memory-access Write in n_tty_set_termios syzbot
2018-04-05 10:31 ` Tetsuo Handa
2018-04-06 10:06   ` Tetsuo Handa
2018-07-17 10:14     ` [PATCH] n_tty: Protect tty->disc_data using refcount Tetsuo Handa
2018-07-24 15:22       ` [PATCH v2] " Tetsuo Handa
2018-07-25  8:06         ` Greg KH
2018-07-25 12:35           ` Tetsuo Handa
2018-08-29 13:53         ` Jiri Slaby
2018-08-29 14:48           ` Jiri Slaby
2018-08-29 15:02           ` Tetsuo Handa

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).