ACK/cmnt: [SRU][Xenial][v2][PATCH 1/1] tty: Prepare for destroying line discipline on hangup
Stefan Bader
stefan.bader at canonical.com
Mon Oct 16 13:03:46 UTC 2017
On 13.10.2017 21:52, Joseph Salisbury wrote:
> From: Peter Hurley <peter at hurleysoftware.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1721065
>
> tty file_operations (read/write/ioctl) wait for the ldisc reference
> indefinitely (until ldisc lifetime events, such as hangup or TIOCSETD,
> finish). Since hangup now destroys the ldisc and does not instance
> another copy, file_operations must now be prepared to receive a NULL
> ldisc reference from tty_ldisc_ref_wait():
>
> CPU 0 CPU 1
> ----- -----
> (*f_op->read)() => tty_read()
> __tty_hangup()
> ...
> f_op = &hung_up_tty_fops;
> ...
> tty_ldisc_hangup()
> tty_ldisc_lock()
> tty_ldisc_kill()
> tty->ldisc = NULL
> tty_ldisc_unlock()
> ld = tty_ldisc_ref_wait()
> /* ld == NULL */
>
> Instead, the action taken now is to return the same value as if the
> tty had been hungup a moment earlier:
>
> CPU 0 CPU 1
> ----- -----
> __tty_hangup()
> ...
> f_op = &hung_up_tty_fops;
> (*f_op->read)() => hung_up_tty_read()
> return 0;
> ...
> tty_ldisc_hangup()
> tty_ldisc_lock()
> tty_ldisc_kill()
> tty->ldisc = NULL
> tty_ldisc_unlock()
>
> Signed-off-by: Peter Hurley <peter at hurleysoftware.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (cherry picked from commit e55afd11a48354c810caf6b6ad4c103016a88230)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
Might be just me but I find a v2 done as a reply getting a bit hard to follow.
Maybe just send the v2 as a fresh series.
-Stefan
> drivers/tty/tty_io.c | 14 ++++++++++++++
> drivers/tty/tty_ldisc.c | 4 ++--
> drivers/tty/vt/selection.c | 2 ++
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 8c0f013..6e3935f 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1066,6 +1066,8 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
> /* We want to wait for the line discipline to sort out in this
> situation */
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return hung_up_tty_read(file, buf, count, ppos);
> if (ld->ops->read)
> i = ld->ops->read(tty, file, buf, count);
> else
> @@ -1241,6 +1243,8 @@ static ssize_t tty_write(struct file *file, const char __user *buf,
> printk(KERN_ERR "tty driver %s lacks a write_room method.\n",
> tty->driver->name);
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return hung_up_tty_write(file, buf, count, ppos);
> if (!ld->ops->write)
> ret = -EIO;
> else
> @@ -2198,6 +2202,8 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
> return 0;
>
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return hung_up_tty_poll(filp, wait);
> if (ld->ops->poll)
> ret = ld->ops->poll(tty, filp, wait);
> tty_ldisc_deref(ld);
> @@ -2287,6 +2293,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
> return -EFAULT;
> tty_audit_tiocsti(tty, ch);
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return -EIO;
> ld->ops->receive_buf(tty, &ch, &mbz, 1);
> tty_ldisc_deref(ld);
> return 0;
> @@ -2679,6 +2687,8 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
> int ret;
>
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return -EIO;
> ret = put_user(ld->ops->num, p);
> tty_ldisc_deref(ld);
> return ret;
> @@ -2976,6 +2986,8 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> return retval;
> }
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return hung_up_tty_ioctl(file, cmd, arg);
> retval = -EINVAL;
> if (ld->ops->ioctl) {
> retval = ld->ops->ioctl(tty, file, cmd, arg);
> @@ -3004,6 +3016,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
> }
>
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return hung_up_tty_compat_ioctl(file, cmd, arg);
> if (ld->ops->compat_ioctl)
> retval = ld->ops->compat_ioctl(tty, file, cmd, arg);
> else
> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index 539dc3c..bc26e48 100644
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -258,8 +258,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
> * against a discipline change, such as an existing ldisc reference
> * (which we check for)
> *
> - * Note: only callable from a file_operations routine (which
> - * guarantees tty->ldisc != NULL when the lock is acquired).
> + * Note: a file_operations routine (read/poll/write) should use this
> + * function to wait for any ldisc lifetime events to finish.
> */
>
> struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
> diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
> index 381a2b1..4dd9dd2 100644
> --- a/drivers/tty/vt/selection.c
> +++ b/drivers/tty/vt/selection.c
> @@ -347,6 +347,8 @@ int paste_selection(struct tty_struct *tty)
> console_unlock();
>
> ld = tty_ldisc_ref_wait(tty);
> + if (!ld)
> + return -EIO; /* ldisc was hung up */
> tty_buffer_lock_exclusive(&vc->port);
>
> add_wait_queue(&vc->paste_wait, &wait);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20171016/dc3ceb76/attachment.sig>
More information about the kernel-team
mailing list