* [PATCH] n_tty: Protect tty->disc_data using refcount.
[not found] ` <35d649ce-0c22-f839-aa4d-19733bf31a9c@I-love.SAKURA.ne.jp>
@ 2018-07-17 10:14 ` Tetsuo Handa
2018-07-24 15:22 ` [PATCH v2] " Tetsuo Handa
0 siblings, 1 reply; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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: 1110 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2018-08-29 19:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <001a113ed31a122ced0568cc6be1@google.com>
[not found] ` <db45a090-593d-23d2-e6b2-ede5b308f02f@I-love.SAKURA.ne.jp>
[not found] ` <35d649ce-0c22-f839-aa4d-19733bf31a9c@I-love.SAKURA.ne.jp>
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).