| Message ID | 20190130154323.11569-1-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,X/B/C] tty: Don't hold ldisc lock in tty_reopen() if ldisc present | expand |
On 30/01/2019 15:43, Stefan Bader wrote: > From: Dmitry Safonov <dima@arista.com> > > BugLink: https://bugs.launchpad.net/bugs/1813873 > > Try to get reference for ldisc during tty_reopen(). > If ldisc present, we don't need to do tty_ldisc_reinit() and lock the > write side for line discipline semaphore. > Effectively, it optimizes fast-path for tty_reopen(), but more > importantly it won't interrupt ongoing IO on the tty as no ldisc change > is needed. > Fixes user-visible issue when tty_reopen() interrupted login process for > user with a long password, observed and reported by Lukas. > > Fixes: c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") > Fixes: 83d817f41070 ("tty: Hold tty_ldisc_lock() during tty_reopen()") > Cc: Jiri Slaby <jslaby@suse.com> > Reported-by: Lukas F. Hartmann <lukas@mntmn.com> > Tested-by: Lukas F. Hartmann <lukas@mntmn.com> > Cc: stable <stable@vger.kernel.org> > Signed-off-by: Dmitry Safonov <dima@arista.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > (cherry picked from commit d3736d82e8169768218ee0ef68718875918091a0) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > > The commits being fixed got requested via LP: #1791758 but > were missing this follow-up. Compile-tested on Xenial. > > -Stefan > > drivers/tty/tty_io.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index bfe9ad85b362..23c6fd238422 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -1256,7 +1256,8 @@ 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 retval; > + struct tty_ldisc *ld; > + int retval = 0; > > if (driver->type == TTY_DRIVER_TYPE_PTY && > driver->subtype == PTY_TYPE_MASTER) > @@ -1268,13 +1269,18 @@ static int tty_reopen(struct tty_struct *tty) > if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) > return -EBUSY; > > - retval = tty_ldisc_lock(tty, 5 * HZ); > - if (retval) > - return retval; > + ld = tty_ldisc_ref_wait(tty); > + if (ld) { > + tty_ldisc_deref(ld); > + } else { > + retval = tty_ldisc_lock(tty, 5 * HZ); > + if (retval) > + return retval; > > - if (!tty->ldisc) > - retval = tty_ldisc_reinit(tty, tty->termios.c_line); > - tty_ldisc_unlock(tty); > + if (!tty->ldisc) > + retval = tty_ldisc_reinit(tty, tty->termios.c_line); > + tty_ldisc_unlock(tty); > + } > > if (retval == 0) > tty->count++; > Cherry pick and a stable fix, so I'm good with this. Acked-by: Colin Ian King <colin.king@canonical.com>
On 2019-01-30 16:43:23 , Stefan Bader wrote: > From: Dmitry Safonov <dima@arista.com> > > BugLink: https://bugs.launchpad.net/bugs/1813873 > > Try to get reference for ldisc during tty_reopen(). > If ldisc present, we don't need to do tty_ldisc_reinit() and lock the > write side for line discipline semaphore. > Effectively, it optimizes fast-path for tty_reopen(), but more > importantly it won't interrupt ongoing IO on the tty as no ldisc change > is needed. > Fixes user-visible issue when tty_reopen() interrupted login process for > user with a long password, observed and reported by Lukas. > > Fixes: c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") > Fixes: 83d817f41070 ("tty: Hold tty_ldisc_lock() during tty_reopen()") > Cc: Jiri Slaby <jslaby@suse.com> > Reported-by: Lukas F. Hartmann <lukas@mntmn.com> > Tested-by: Lukas F. Hartmann <lukas@mntmn.com> > Cc: stable <stable@vger.kernel.org> > Signed-off-by: Dmitry Safonov <dima@arista.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > (cherry picked from commit d3736d82e8169768218ee0ef68718875918091a0) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > > The commits being fixed got requested via LP: #1791758 but > were missing this follow-up. Compile-tested on Xenial. > > -Stefan > > drivers/tty/tty_io.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index bfe9ad85b362..23c6fd238422 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -1256,7 +1256,8 @@ 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 retval; > + struct tty_ldisc *ld; > + int retval = 0; > > if (driver->type == TTY_DRIVER_TYPE_PTY && > driver->subtype == PTY_TYPE_MASTER) > @@ -1268,13 +1269,18 @@ static int tty_reopen(struct tty_struct *tty) > if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) > return -EBUSY; > > - retval = tty_ldisc_lock(tty, 5 * HZ); > - if (retval) > - return retval; > + ld = tty_ldisc_ref_wait(tty); > + if (ld) { > + tty_ldisc_deref(ld); > + } else { > + retval = tty_ldisc_lock(tty, 5 * HZ); > + if (retval) > + return retval; > > - if (!tty->ldisc) > - retval = tty_ldisc_reinit(tty, tty->termios.c_line); > - tty_ldisc_unlock(tty); > + if (!tty->ldisc) > + retval = tty_ldisc_reinit(tty, tty->termios.c_line); > + tty_ldisc_unlock(tty); > + } > > if (retval == 0) > tty->count++; Acked-by: Khalid Elmously <khalid.elmously@canonical.com>
On 2019-01-30 16:43:23 , Stefan Bader wrote: > From: Dmitry Safonov <dima@arista.com> > > BugLink: https://bugs.launchpad.net/bugs/1813873 > > Try to get reference for ldisc during tty_reopen(). > If ldisc present, we don't need to do tty_ldisc_reinit() and lock the > write side for line discipline semaphore. > Effectively, it optimizes fast-path for tty_reopen(), but more > importantly it won't interrupt ongoing IO on the tty as no ldisc change > is needed. > Fixes user-visible issue when tty_reopen() interrupted login process for > user with a long password, observed and reported by Lukas. > > Fixes: c96cf923a98d ("tty: Don't block on IO when ldisc change is pending") > Fixes: 83d817f41070 ("tty: Hold tty_ldisc_lock() during tty_reopen()") > Cc: Jiri Slaby <jslaby@suse.com> > Reported-by: Lukas F. Hartmann <lukas@mntmn.com> > Tested-by: Lukas F. Hartmann <lukas@mntmn.com> > Cc: stable <stable@vger.kernel.org> > Signed-off-by: Dmitry Safonov <dima@arista.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > (cherry picked from commit d3736d82e8169768218ee0ef68718875918091a0) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > > The commits being fixed got requested via LP: #1791758 but > were missing this follow-up. Compile-tested on Xenial. > > -Stefan > > drivers/tty/tty_io.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c > index bfe9ad85b362..23c6fd238422 100644 > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -1256,7 +1256,8 @@ 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 retval; > + struct tty_ldisc *ld; > + int retval = 0; > > if (driver->type == TTY_DRIVER_TYPE_PTY && > driver->subtype == PTY_TYPE_MASTER) > @@ -1268,13 +1269,18 @@ static int tty_reopen(struct tty_struct *tty) > if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) > return -EBUSY; > > - retval = tty_ldisc_lock(tty, 5 * HZ); > - if (retval) > - return retval; > + ld = tty_ldisc_ref_wait(tty); > + if (ld) { > + tty_ldisc_deref(ld); > + } else { > + retval = tty_ldisc_lock(tty, 5 * HZ); > + if (retval) > + return retval; > > - if (!tty->ldisc) > - retval = tty_ldisc_reinit(tty, tty->termios.c_line); > - tty_ldisc_unlock(tty); > + if (!tty->ldisc) > + retval = tty_ldisc_reinit(tty, tty->termios.c_line); > + tty_ldisc_unlock(tty); > + } > > if (retval == 0) > tty->count++; > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index bfe9ad85b362..23c6fd238422 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1256,7 +1256,8 @@ 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 retval; + struct tty_ldisc *ld; + int retval = 0; if (driver->type == TTY_DRIVER_TYPE_PTY && driver->subtype == PTY_TYPE_MASTER) @@ -1268,13 +1269,18 @@ static int tty_reopen(struct tty_struct *tty) if (test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) return -EBUSY; - retval = tty_ldisc_lock(tty, 5 * HZ); - if (retval) - return retval; + ld = tty_ldisc_ref_wait(tty); + if (ld) { + tty_ldisc_deref(ld); + } else { + retval = tty_ldisc_lock(tty, 5 * HZ); + if (retval) + return retval; - if (!tty->ldisc) - retval = tty_ldisc_reinit(tty, tty->termios.c_line); - tty_ldisc_unlock(tty); + if (!tty->ldisc) + retval = tty_ldisc_reinit(tty, tty->termios.c_line); + tty_ldisc_unlock(tty); + } if (retval == 0) tty->count++;