From: Thomas Veerman Date: Wed, 4 Apr 2012 13:46:34 +0000 (+0000) Subject: TTY: don't allow multiple readers on tty minor X-Git-Tag: v3.2.1~593 X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/index.html?a=commitdiff_plain;h=ca7a466f48db3d64aed7b5c082e7f3634bbd526a;p=minix.git TTY: don't allow multiple readers on tty minor TTY has no way of keeping track of multiple readers for a tty minor device. Instead, it stores a read request for the last reader only. Consequently, the first ("overwritten") reader gets stuck on a read request that's never going to be finished. Also, the overwriting causes a grant mismatch in VFS when TTY returns a reply for the second reader. This patch is a work around for the actual problem (i.e., keeping track of multiple readers). It checks whether there is a read operation in progress and returns an error if it is --preventing that reader from getting overwritten and stuck. It fixes a bug triggered by executing 'top | more' and pressing the space bar for a while (easily reproducable in a VM, not on hardware). --- diff --git a/drivers/tty/keyboard.c b/drivers/tty/keyboard.c index 65f585b0d..534f3e337 100644 --- a/drivers/tty/keyboard.c +++ b/drivers/tty/keyboard.c @@ -399,7 +399,8 @@ message *m; { int n, r; - if (kbdp->avail && kbdp->req_size && m->m_source == kbdp->incaller) + if (kbdp->avail && kbdp->req_size && m->m_source == kbdp->incaller && + kbdp->req_grant != GRANT_INVALID) { /* Handle read request */ n= kbdp->avail; @@ -423,6 +424,7 @@ message *m; m->REP_ENDPT= kbdp->req_proc; m->REP_IO_GRANT= kbdp->req_grant; m->REP_STATUS= r; + kbdp->req_grant = GRANT_INVALID; return 1; } if (kbdp->avail && (kbdp->select_ops & SEL_RD) && diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index b0123f4f2..6adb2af8d 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -101,6 +101,10 @@ void do_pty(tty_t *tp, message *m_ptr) r = EINVAL; break; } + if (pp->rdgrant != GRANT_INVALID) { + r = ENOBUFS; + break; + } pp->rdsendreply = TRUE; pp->rdcaller = m_ptr->m_source; pp->rdproc = m_ptr->USER_ENDPT; @@ -110,6 +114,7 @@ void do_pty(tty_t *tp, message *m_ptr) pty_start(pp); handle_events(tp); if (pp->rdleft == 0) { + pp->rdgrant = GRANT_INVALID; return; /* already done */ } @@ -133,6 +138,10 @@ void do_pty(tty_t *tp, message *m_ptr) r = EINVAL; break; } + if (pp->wrgrant != GRANT_INVALID) { + r = ENOBUFS; + break; + } pp->wrsendreply = TRUE; pp->wrcaller = m_ptr->m_source; pp->wrproc = m_ptr->USER_ENDPT; @@ -141,6 +150,7 @@ void do_pty(tty_t *tp, message *m_ptr) pp->wrleft = m_ptr->COUNT; handle_events(tp); if (pp->wrleft == 0) { + pp->wrgrant = GRANT_INVALID; return; /* already done */ } @@ -177,11 +187,13 @@ void do_pty(tty_t *tp, message *m_ptr) /* Cancel a read from a PTY. */ r = pp->rdcum > 0 ? pp->rdcum : EAGAIN; pp->rdleft = pp->rdcum = 0; + pp->rdgrant = GRANT_INVALID; } if (m_ptr->USER_ENDPT == pp->wrproc) { /* Cancel a write to a PTY. */ r = pp->wrcum > 0 ? pp->wrcum : EAGAIN; pp->wrleft = pp->wrcum = 0; + pp->wrgrant = GRANT_INVALID; } break; @@ -471,6 +483,8 @@ void pty_init(tty_t *tp) pp = tp->tty_priv = &pty_table[line]; pp->tty = tp; pp->select_ops = 0; + pp->rdgrant = GRANT_INVALID; + pp->wrgrant = GRANT_INVALID; /* Set up output queue. */ pp->ohead = pp->otail = pp->obuf; @@ -503,8 +517,8 @@ int pty_status(message *m_ptr) m_ptr->REP_ENDPT = pp->rdproc; m_ptr->REP_IO_GRANT = pp->rdgrant; m_ptr->REP_STATUS = pp->rdcum; - pp->rdleft = pp->rdcum = 0; + pp->rdgrant = GRANT_INVALID; event_found = 1; break; } @@ -522,6 +536,7 @@ int pty_status(message *m_ptr) m_ptr->REP_STATUS = pp->wrcum; pp->wrleft = pp->wrcum = 0; + pp->wrgrant = GRANT_INVALID; event_found = 1; break; } diff --git a/drivers/tty/tty.c b/drivers/tty/tty.c index 7ed0b97e3..67396b51c 100644 --- a/drivers/tty/tty.c +++ b/drivers/tty/tty.c @@ -385,8 +385,9 @@ message *m_ptr; tp->tty_inleft = tp->tty_incum = 0; tp->tty_inrevived = 0; /* unmark revive event */ - event_found = 1; - break; + tp->tty_ingrant = GRANT_INVALID; + event_found = 1; + break; } else if (tp->tty_outrevived && tp->tty_outcaller == m_ptr->m_source) { @@ -398,18 +399,21 @@ message *m_ptr; tp->tty_outcum = 0; tp->tty_outrevived = 0; /* unmark revive event */ - event_found = 1; - break; + tp->tty_outgrant = GRANT_INVALID; + event_found = 1; + break; } else if (tp->tty_iorevived && tp->tty_iocaller == m_ptr->m_source) { + /* Suspended request finished. Send a REVIVE. */ m_ptr->m_type = DEV_REVIVE; m_ptr->REP_ENDPT = tp->tty_ioproc; m_ptr->REP_IO_GRANT = tp->tty_iogrant; m_ptr->REP_STATUS = tp->tty_iostatus; tp->tty_iorevived = 0; /* unmark revive event */ - event_found = 1; - break; + tp->tty_iogrant = GRANT_INVALID; + event_found = 1; + break; } } @@ -451,6 +455,12 @@ register message *m_ptr; /* pointer to message sent to the task */ } else if (m_ptr->COUNT <= 0) { r = EINVAL; + } else if (tp->tty_ingrant != GRANT_INVALID) { + /* This is actually a fundamental problem with TTY; it can handle + * only one reader per minor device. If we don't return an error, + * we'll overwrite the previous reader and that process will get + * stuck forever. */ + r = ENOBUFS; } else { /* Copy information from the message to the tty struct. */ tp->tty_inrepcode = TASK_REPLY; @@ -484,8 +494,6 @@ register message *m_ptr; /* pointer to message sent to the task */ /* ...then go back for more. */ handle_events(tp); if (tp->tty_inleft == 0) { - if (tp->tty_select_ops) - select_retry(tp); return; /* already done */ } @@ -529,7 +537,7 @@ register message *m_ptr; /* pointer to message sent to the task */ /* Try to write. */ handle_events(tp); - if (tp->tty_outleft == 0) + if (tp->tty_outleft == 0) return; /* already done */ /* None or not all the bytes could be written, so suspend the @@ -772,12 +780,14 @@ message *m_ptr; /* pointer to message sent to task */ tty_icancel(tp); r = tp->tty_incum > 0 ? tp->tty_incum : EAGAIN; tp->tty_inleft = tp->tty_incum = tp->tty_inrevived = 0; + tp->tty_ingrant = GRANT_INVALID; } if ((mode & W_BIT) && tp->tty_outleft != 0 && proc_nr == tp->tty_outproc && tp->tty_outgrant == (cp_grant_id_t) m_ptr->IO_GRANT) { /* Process was writing when killed. Clean up output. */ r = tp->tty_outcum > 0 ? tp->tty_outcum : EAGAIN; tp->tty_outleft = tp->tty_outcum = tp->tty_outrevived = 0; + tp->tty_outgrant = GRANT_INVALID; } if (tp->tty_ioreq != 0 && proc_nr == tp->tty_ioproc) { /* Process was waiting for output to drain. */ @@ -873,6 +883,8 @@ tty_t *tp; /* TTY to check for events. */ tty_reply(tp->tty_inrepcode, tp->tty_incaller, tp->tty_inproc, tp->tty_incum); tp->tty_inleft = tp->tty_incum = 0; + tp->tty_inrevived = 0; + tp->tty_ingrant = GRANT_INVALID; } } if (tp->tty_select_ops) @@ -953,6 +965,8 @@ register tty_t *tp; /* pointer to terminal to read from */ tty_reply(tp->tty_inrepcode, tp->tty_incaller, tp->tty_inproc, tp->tty_incum); tp->tty_inleft = tp->tty_incum = 0; + tp->tty_inrevived = 0; + tp->tty_ingrant = GRANT_INVALID; } } } @@ -1551,6 +1565,7 @@ static void tty_init() tp->tty_intail = tp->tty_inhead = tp->tty_inbuf; tp->tty_min = 1; + tp->tty_ingrant = tp->tty_outgrant = tp->tty_iogrant = GRANT_INVALID; tp->tty_termios = termios_defaults; tp->tty_icancel = tp->tty_ocancel = tp->tty_ioctl = tp->tty_close = tty_devnop;