KASAN: user-memory-access Write in n_tty_set_termios

[syzbot]

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+3aa978...@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 syzk...@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.

[Tetsuo Handa]

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;
}
----------------------------------------

[Tetsuo Handa]

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 */

[Tetsuo Handa]

From e8360c16fd07985686bcfb388364103f35a6523a Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin...@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...@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

[Tetsuo Handa]

From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin...@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...@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

@@ -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,

[Greg KH]

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

[Tetsuo Handa]

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.

[Jiri Slaby]

On 07/24/2018, 05:22 PM, Tetsuo Handa wrote:
> From 118c64e86641a97d44dec39e313a95b12d9bc3b2 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin...@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

[Jiri Slaby]

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

> thanks,--
js
suse labs

[Tetsuo Handa]

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