Commit 1ee33b1c authored by Tetsuo Handa's avatar Tetsuo Handa Committed by Greg Kroah-Hartman
Browse files

tty: n_hdlc: make n_hdlc_tty_wakeup() asynchronous 

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: default avatarsyzbot <syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com>
Cc: stable <stable@vger.kernel.org>
Analyzed-by: default avatarFabio M. De Francesco <fmdefrancesco@gmail.com>
Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Confirmed-by: default avatarJiri Slaby <jirislaby@kernel.org>
Reviewed-by: default avatarFabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: default avatarTetsuo 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: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 0fcfb00b

drivers/tty/n_hdlc.c

+22 −1
Original line number Diff line number Diff line
@@ -140,6 +140,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;

};



/*
@@ -154,6 +156,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;
@@ -210,6 +213,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);
@@ -241,6 +246,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;
@@ -334,6 +341,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
@@ -344,7 +365,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() */



/**