Messages in this thread | | | Date | Wed, 1 Dec 2021 22:40:38 +0900 | Subject | Re: [PATCH] tty: vt: make do_con_write() no-op if IRQ is disabled | From | Tetsuo Handa <> |
| |
What do you think? Can we apply this?
On 2021/11/19 23:55, Tetsuo Handa wrote: > syzbot is reporting that an unprivileged user who logged in from tty > console can crash the system using a reproducer shown below [1], for > commit f9e053dcfc02b0ad ("tty: Serialize tty flow control changes with > flow_lock") changed __start_tty() from schedulable context to atomic > context without realizing that drivers/tty/n_hdlc.c is calling > do_con_write() (which waits for console semaphore) from __start_tty(). > > ---------- > #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 */ > } > } > ---------- > > While we would need to bring __start_tty() back to schedulable context > in order to be able to call do_con_write() from ioctl(TCXONC, TCOON), > changes required might be complicated; an oversight will result in a > new sleep from atomic context bug or a new race window bug. Therefore, > firstly let's fix crash problem by protecting do_con_write() side. > > Link: https://syzkaller.appspot.com/bug?extid=5f47a8cea6a12b77a876 [1] > Reported-by: syzbot <syzbot+5f47a8cea6a12b77a876@syzkaller.appspotmail.com> > Analyzed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > Fixes: f9e053dcfc02b0ad ("tty: Serialize tty flow control changes with flow_lock") > --- > drivers/tty/vt/vt.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 7359c3e80d63..efa16f51c978 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -2902,7 +2902,7 @@ static int do_con_write(struct tty_struct *tty, const unsigned char *buf, int co > struct vt_notifier_param param; > bool rescan; > > - if (in_interrupt()) > + if (in_interrupt() || irqs_disabled()) > return count; > > console_lock(); > @@ -3358,7 +3358,7 @@ static void con_flush_chars(struct tty_struct *tty) > { > struct vc_data *vc; > > - if (in_interrupt()) /* from flush_to_ldisc */ > + if (in_interrupt() || irqs_disabled()) /* from flush_to_ldisc */ > return; > > /* if we race with con_close(), vt may be null */ >
| |