[PATCH Trusty SRU] tty: Fix pty master poll() after slave closes v2
Andy Whitcroft
apw at canonical.com
Tue Sep 1 08:05:33 UTC 2015
On Fri, Aug 28, 2015 at 09:47:49AM -0600, tim.gardner at canonical.com wrote:
> From: Francesco Ruggeri <fruggeri at aristanetworks.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1397976
>
> Commit f95499c3030f ("n_tty: Don't wait for buffer work in read() loop")
> introduces a race window where a pty master can be signalled that the pty
> slave was closed before all the data that the slave wrote is delivered.
> Commit f8747d4a466a ("tty: Fix pty master read() after slave closes") fixed the
> problem in case of n_tty_read, but the problem still exists for n_tty_poll.
> This can be seen by running 'for ((i=0; i<100;i++));do ./test.py ;done'
> where test.py is:
>
> import os, select, pty
>
> (pid, pty_fd) = pty.fork()
>
> if pid == 0:
> os.write(1, 'This string should be received by parent')
> else:
> poller = select.epoll()
> poller.register( pty_fd, select.EPOLLIN )
> ready = poller.poll( 1 * 1000 )
> for fd, events in ready:
> if not events & select.EPOLLIN:
> print 'missed POLLIN event'
> else:
> print os.read(fd, 100)
> poller.close()
>
> The string from the slave is missed several times.
> This patch takes the same approach as the fix for read and special cases
> this condition for poll.
> Tested on 3.16.
>
> Signed-off-by: Francesco Ruggeri <fruggeri at arista.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (back ported from commit c4dc304677e8d566572c4738d95c48be150c6606)
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
>
> Conflicts:
> drivers/tty/n_tty.c
> ---
> drivers/tty/n_tty.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index ba9270b..ca045db 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -2477,12 +2477,17 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
>
> poll_wait(file, &tty->read_wait, wait);
> poll_wait(file, &tty->write_wait, wait);
> + if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
> + mask |= POLLHUP;
> if (input_available_p(tty, TIME_CHAR(tty) ? 0 : MIN_CHAR(tty)))
> mask |= POLLIN | POLLRDNORM;
I believe the intent of this change to be such that on detecting a HUP
event we flush the line discipline buffers down and then recheck for input.
That it would trigger a repeat of this input_available_p() incantation
after a tty_flush_to_ldisc() ...
> + else if (mask & POLLHUP) {
> + tty_flush_to_ldisc(tty);
> + if (input_available_p(tty, 1))
> + mask |= POLLIN | POLLRDNORM;
... so I expecting this second ones parameters to match the first.
If I am reading the history right the semantics of input_available_p()
changes in the commit below from a size to a boolean "poll", which falls
between where we are at and the commit being backported:
commit eafbe67f84761d787802e5113d895a316b6292fe
Author: Peter Hurley <peter at hurleysoftware.com>
Date: Mon Dec 2 14:24:45 2013 -0500
n_tty: Refactor input_available_p() by call site
> + }
> if (tty->packet && tty->link->ctrl_status)
> mask |= POLLPRI | POLLIN | POLLRDNORM;
> - if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
> - mask |= POLLHUP;
> if (tty_hung_up_p(file))
> mask |= POLLHUP;
> if (!(mask & (POLLHUP | POLLIN | POLLRDNORM))) {
-apw
More information about the kernel-team
mailing list