]> Zhao Yanbai Git Server - minix.git/commitdiff
TTY: don't allow multiple readers on tty minor
authorThomas Veerman <thomas@minix3.org>
Wed, 4 Apr 2012 13:46:34 +0000 (13:46 +0000)
committerThomas Veerman <thomas@minix3.org>
Fri, 13 Apr 2012 13:22:13 +0000 (13:22 +0000)
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).

drivers/tty/keyboard.c
drivers/tty/pty.c
drivers/tty/tty.c

index 65f585b0d215d3a4b198c1de00a0d12fbe8ca45a..534f3e33718d7cb3e09e2b7cd00dd58614e8ab8d 100644 (file)
@@ -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) &&
index b0123f4f2a11d2b0d9fcf30fcbd5eafc8575fbc3..6adb2af8dcd882b4b80b525d94f148602f42c6e0 100644 (file)
@@ -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;
                }
index 7ed0b97e3b9e3a3be3c53339cc002c80422e0f01..67396b51c201c1eaf5ab19dca9bcaaf921144639 100644 (file)
@@ -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;