Skip to content

Commit

Permalink
tty: n_hdlc: make n_hdlc_tty_wakeup() asynchronous
Browse files Browse the repository at this point in the history
commit 1ee33b1 upstream.

syzbot is reporting that an unprivileged user who logged in from tty
console can crash the system using a reproducer shown below [1], for
n_hdlc_tty_wakeup() is synchronously calling n_hdlc_send_frames().

----------
  #include <sys/ioctl.h>
  #include <unistd.h>

  int main(int argc, char *argv[])
  {
    const int disc = 0xd;

    ioctl(1, TIOCSETD, &disc);
    while (1) {
      ioctl(1, TCXONC, 0);
      write(1, "", 1);
      ioctl(1, TCXONC, 1); /* Kernel panic - not syncing: scheduling while atomic */
    }
  }
----------

Linus suspected that "struct tty_ldisc"->ops->write_wakeup() must not
sleep, and Jiri confirmed it from include/linux/tty_ldisc.h. Thus, defer
n_hdlc_send_frames() from n_hdlc_tty_wakeup() to a WQ context like
net/nfc/nci/uart.c does.

Link: https://syzkaller.appspot.com/bug?extid=5f47a8cea6a12b77a876 [1]
Reported-by: syzbot <syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com>
Cc: stable <stable@vger.kernel.org>
Analyzed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Confirmed-by: Jiri Slaby <jirislaby@kernel.org>
Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Link: https://lore.kernel.org/r/40de8b7e-a3be-4486-4e33-1b1d1da452f8@i-love.sakura.ne.jp
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Tetsuo Handa authored and gregkh committed Dec 22, 2021
1 parent 9439fab commit fd623e1
Showing 1 changed file with 22 additions and 1 deletion.
23 changes: 22 additions & 1 deletion drivers/tty/n_hdlc.c
Expand Up @@ -139,6 +139,8 @@ struct n_hdlc {
struct n_hdlc_buf_list rx_buf_list;
struct n_hdlc_buf_list tx_free_buf_list;
struct n_hdlc_buf_list rx_free_buf_list;
struct work_struct write_work;
struct tty_struct *tty_for_write_work;
};

/*
Expand All @@ -153,6 +155,7 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *list);
/* Local functions */

static struct n_hdlc *n_hdlc_alloc(void);
static void n_hdlc_tty_write_work(struct work_struct *work);

/* max frame size for memory allocations */
static int maxframe = 4096;
Expand Down Expand Up @@ -209,6 +212,8 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
wake_up_interruptible(&tty->read_wait);
wake_up_interruptible(&tty->write_wait);

cancel_work_sync(&n_hdlc->write_work);

n_hdlc_free_buf_list(&n_hdlc->rx_free_buf_list);
n_hdlc_free_buf_list(&n_hdlc->tx_free_buf_list);
n_hdlc_free_buf_list(&n_hdlc->rx_buf_list);
Expand Down Expand Up @@ -240,6 +245,8 @@ static int n_hdlc_tty_open(struct tty_struct *tty)
return -ENFILE;
}

INIT_WORK(&n_hdlc->write_work, n_hdlc_tty_write_work);
n_hdlc->tty_for_write_work = tty;
tty->disc_data = n_hdlc;
tty->receive_room = 65536;

Expand Down Expand Up @@ -333,6 +340,20 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty)
goto check_again;
} /* end of n_hdlc_send_frames() */

/**
* n_hdlc_tty_write_work - Asynchronous callback for transmit wakeup
* @work: pointer to work_struct
*
* Called when low level device driver can accept more send data.
*/
static void n_hdlc_tty_write_work(struct work_struct *work)
{
struct n_hdlc *n_hdlc = container_of(work, struct n_hdlc, write_work);
struct tty_struct *tty = n_hdlc->tty_for_write_work;

n_hdlc_send_frames(n_hdlc, tty);
} /* end of n_hdlc_tty_write_work() */

/**
* n_hdlc_tty_wakeup - Callback for transmit wakeup
* @tty: pointer to associated tty instance data
Expand All @@ -343,7 +364,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
{
struct n_hdlc *n_hdlc = tty->disc_data;

n_hdlc_send_frames(n_hdlc, tty);
schedule_work(&n_hdlc->write_work);
} /* end of n_hdlc_tty_wakeup() */

/**
Expand Down

0 comments on commit fd623e1

Please sign in to comment.