n_tty: Access echo_* variables carefully.
syzbot is reporting stalls at __process_echoes() [1]. This is because since ldata->echo_commit < ldata->echo_tail becomes true for some reason, the discard loop is serving as almost infinite loop. This patch tries to avoid falling into ldata->echo_commit < ldata->echo_tail situation by making access to echo_* variables more carefully. Since reset_buffer_flags() is called without output_lock held, it should not touch echo_* variables. And omit a call to reset_buffer_flags() from n_tty_open() by using vzalloc(). Since add_echo_byte() is called without output_lock held, it needs memory barrier between storing into echo_buf[] and incrementing echo_head counter. echo_buf() needs corresponding memory barrier before reading echo_buf[]. Lack of handling the possibility of not-yet-stored multi-byte operation might be the reason of falling into ldata->echo_commit < ldata->echo_tail situation, for if I do WARN_ON(ldata->echo_commit == tail + 1) prior to echo_buf(ldata, tail + 1), the WARN_ON() fires. Also, explicitly masking with buffer for the former "while" loop, and use ldata->echo_commit > tail for the latter "while" loop. [1] https://syzkaller.appspot.com/bug?id=17f23b094cd80df750e5b0f8982c521ee6bcbf40 Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reported-by: syzbot <syzbot+108696293d7a21ab688f@syzkaller.appspotmail.com> Cc: Peter Hurley <peter@hurleysoftware.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
3d63b7e4ae
commit
ebec3f8f52
|
@ -143,6 +143,7 @@ static inline unsigned char *read_buf_addr(struct n_tty_data *ldata, size_t i)
|
|||
|
||||
static inline unsigned char echo_buf(struct n_tty_data *ldata, size_t i)
|
||||
{
|
||||
smp_rmb(); /* Matches smp_wmb() in add_echo_byte(). */
|
||||
return ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
|
||||
}
|
||||
|
||||
|
@ -318,9 +319,7 @@ static inline void put_tty_queue(unsigned char c, struct n_tty_data *ldata)
|
|||
static void reset_buffer_flags(struct n_tty_data *ldata)
|
||||
{
|
||||
ldata->read_head = ldata->canon_head = ldata->read_tail = 0;
|
||||
ldata->echo_head = ldata->echo_tail = ldata->echo_commit = 0;
|
||||
ldata->commit_head = 0;
|
||||
ldata->echo_mark = 0;
|
||||
ldata->line_start = 0;
|
||||
|
||||
ldata->erasing = 0;
|
||||
|
@ -619,12 +618,19 @@ static size_t __process_echoes(struct tty_struct *tty)
|
|||
old_space = space = tty_write_room(tty);
|
||||
|
||||
tail = ldata->echo_tail;
|
||||
while (ldata->echo_commit != tail) {
|
||||
while (MASK(ldata->echo_commit) != MASK(tail)) {
|
||||
c = echo_buf(ldata, tail);
|
||||
if (c == ECHO_OP_START) {
|
||||
unsigned char op;
|
||||
int no_space_left = 0;
|
||||
|
||||
/*
|
||||
* Since add_echo_byte() is called without holding
|
||||
* output_lock, we might see only portion of multi-byte
|
||||
* operation.
|
||||
*/
|
||||
if (MASK(ldata->echo_commit) == MASK(tail + 1))
|
||||
goto not_yet_stored;
|
||||
/*
|
||||
* If the buffer byte is the start of a multi-byte
|
||||
* operation, get the next byte, which is either the
|
||||
|
@ -636,6 +642,8 @@ static size_t __process_echoes(struct tty_struct *tty)
|
|||
unsigned int num_chars, num_bs;
|
||||
|
||||
case ECHO_OP_ERASE_TAB:
|
||||
if (MASK(ldata->echo_commit) == MASK(tail + 2))
|
||||
goto not_yet_stored;
|
||||
num_chars = echo_buf(ldata, tail + 2);
|
||||
|
||||
/*
|
||||
|
@ -730,7 +738,8 @@ static size_t __process_echoes(struct tty_struct *tty)
|
|||
/* If the echo buffer is nearly full (so that the possibility exists
|
||||
* of echo overrun before the next commit), then discard enough
|
||||
* data at the tail to prevent a subsequent overrun */
|
||||
while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
|
||||
while (ldata->echo_commit > tail &&
|
||||
ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
|
||||
if (echo_buf(ldata, tail) == ECHO_OP_START) {
|
||||
if (echo_buf(ldata, tail + 1) == ECHO_OP_ERASE_TAB)
|
||||
tail += 3;
|
||||
|
@ -740,6 +749,7 @@ static size_t __process_echoes(struct tty_struct *tty)
|
|||
tail++;
|
||||
}
|
||||
|
||||
not_yet_stored:
|
||||
ldata->echo_tail = tail;
|
||||
return old_space - space;
|
||||
}
|
||||
|
@ -750,6 +760,7 @@ static void commit_echoes(struct tty_struct *tty)
|
|||
size_t nr, old, echoed;
|
||||
size_t head;
|
||||
|
||||
mutex_lock(&ldata->output_lock);
|
||||
head = ldata->echo_head;
|
||||
ldata->echo_mark = head;
|
||||
old = ldata->echo_commit - ldata->echo_tail;
|
||||
|
@ -758,10 +769,12 @@ static void commit_echoes(struct tty_struct *tty)
|
|||
* is over the threshold (and try again each time another
|
||||
* block is accumulated) */
|
||||
nr = head - ldata->echo_tail;
|
||||
if (nr < ECHO_COMMIT_WATERMARK || (nr % ECHO_BLOCK > old % ECHO_BLOCK))
|
||||
if (nr < ECHO_COMMIT_WATERMARK ||
|
||||
(nr % ECHO_BLOCK > old % ECHO_BLOCK)) {
|
||||
mutex_unlock(&ldata->output_lock);
|
||||
return;
|
||||
}
|
||||
|
||||
mutex_lock(&ldata->output_lock);
|
||||
ldata->echo_commit = head;
|
||||
echoed = __process_echoes(tty);
|
||||
mutex_unlock(&ldata->output_lock);
|
||||
|
@ -812,7 +825,9 @@ static void flush_echoes(struct tty_struct *tty)
|
|||
|
||||
static inline void add_echo_byte(unsigned char c, struct n_tty_data *ldata)
|
||||
{
|
||||
*echo_buf_addr(ldata, ldata->echo_head++) = c;
|
||||
*echo_buf_addr(ldata, ldata->echo_head) = c;
|
||||
smp_wmb(); /* Matches smp_rmb() in echo_buf(). */
|
||||
ldata->echo_head++;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -1881,30 +1896,21 @@ static int n_tty_open(struct tty_struct *tty)
|
|||
struct n_tty_data *ldata;
|
||||
|
||||
/* Currently a malloc failure here can panic */
|
||||
ldata = vmalloc(sizeof(*ldata));
|
||||
ldata = vzalloc(sizeof(*ldata));
|
||||
if (!ldata)
|
||||
goto err;
|
||||
return -ENOMEM;
|
||||
|
||||
ldata->overrun_time = jiffies;
|
||||
mutex_init(&ldata->atomic_read_lock);
|
||||
mutex_init(&ldata->output_lock);
|
||||
|
||||
tty->disc_data = ldata;
|
||||
reset_buffer_flags(tty->disc_data);
|
||||
ldata->column = 0;
|
||||
ldata->canon_column = 0;
|
||||
ldata->num_overrun = 0;
|
||||
ldata->no_room = 0;
|
||||
ldata->lnext = 0;
|
||||
tty->closing = 0;
|
||||
/* indicate buffer work may resume */
|
||||
clear_bit(TTY_LDISC_HALTED, &tty->flags);
|
||||
n_tty_set_termios(tty, NULL);
|
||||
tty_unthrottle(tty);
|
||||
|
||||
return 0;
|
||||
err:
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
static inline int input_available_p(struct tty_struct *tty, int poll)
|
||||
|
|
Loading…
Reference in a new issue