kernel - Fix pty registration, memory leak
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 13 Aug 2010 01:55:20 +0000 (18:55 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 13 Aug 2010 01:58:48 +0000 (18:58 -0700)
* Properly unregister the pt_ioctl before freeing it.

* Free the pt_ioctl structure after destroying the device, fixing
  a memory leak.

* Fix races in the sysctl_proc which iterates the tty list by using
  a marker.

sys/kern/tty.c
sys/kern/tty_pty.c
sys/sys/tty.h

index 6c24a89..5c2a701 100644 (file)
@@ -210,7 +210,7 @@ uint64_t tk_rawcc;
 /*
  * list of struct tty where pstat(8) can pick it up with sysctl
  */
-static SLIST_HEAD(, tty) tty_list;
+static TAILQ_HEAD(, tty) tty_list = TAILQ_HEAD_INITIALIZER(tty_list);
 
 /*
  * Initial open of tty, or (re)entry to standard tty line discipline.
@@ -257,7 +257,7 @@ ttyclose(struct tty *tp)
        tp->t_gen++;
        tp->t_line = TTYDISC;
        ttyclearsession(tp);
-       tp->t_state = 0;
+       tp->t_state &= TS_REGISTERED;   /* clear all bits except */
        crit_exit();
        return (0);
 }
@@ -2576,42 +2576,49 @@ ttymalloc(struct tty *tp)
         return (tp);
 }
 
-#if 0
-/*
- * Free a tty struct.  Clists in the struct should have been freed by
- * ttyclose().
- *
- * XXX not yet usable: since we support a half-closed state and do not
- * ref count the tty reference from the session, it is possible for a 
- * session to hold a ref on the tty.  See TTY_DO_FULL_CLOSE.
- */
 void
-ttyfree(struct tty *tp)
+ttyunregister(struct tty *tp)
 {
-        kfree(tp, M_TTYS);
+       KKASSERT(ISSET(tp->t_state, TS_REGISTERED));
+       CLR(tp->t_state, TS_REGISTERED);
+       TAILQ_REMOVE(&tty_list, tp, t_list);
 }
-#endif /* 0 */
 
 void
 ttyregister(struct tty *tp)
 {
-       SLIST_INSERT_HEAD(&tty_list, tp, t_list);
+       KKASSERT(!ISSET(tp->t_state, TS_REGISTERED));
+       SET(tp->t_state, TS_REGISTERED);
+       TAILQ_INSERT_HEAD(&tty_list, tp, t_list);
 }
 
 static int
 sysctl_kern_ttys(SYSCTL_HANDLER_ARGS)
 {
        int error;
-       struct tty *tp, t;
-       SLIST_FOREACH(tp, &tty_list, t_list) {
+       struct tty *tp;
+       struct tty t;
+       struct tty marker;
+
+       bzero(&marker, sizeof(marker));
+       marker.t_state = TS_MARKER;
+       error = 0;
+
+       TAILQ_INSERT_HEAD(&tty_list, &marker, t_list);
+       while ((tp = TAILQ_NEXT(&marker, t_list)) != NULL) {
+               TAILQ_REMOVE(&tty_list, &marker, t_list);
+               TAILQ_INSERT_AFTER(&tty_list, tp, &marker, t_list);
+               if (tp->t_state & TS_MARKER)
+                       continue;
                t = *tp;
                if (t.t_dev)
                        t.t_dev = (cdev_t)(uintptr_t)dev2udev(t.t_dev);
                error = SYSCTL_OUT(req, (caddr_t)&t, sizeof(t));
                if (error)
-                       return (error);
+                       break;
        }
-       return (0);
+       TAILQ_REMOVE(&tty_list, &marker, t_list);
+       return (error);
 }
 
 SYSCTL_PROC(_kern, OID_AUTO, ttys, CTLTYPE_OPAQUE|CTLFLAG_RD,
index b21907f..ddf0287 100644 (file)
@@ -317,8 +317,10 @@ ptsclose(struct dev_close_args *ap)
        struct tty *tp;
        struct pt_ioctl *pti = dev->si_drv1;
        int err;
+       int uminor_no;
 
        tp = dev->si_tty;
+       uminor_no = dev->si_uminor;
        err = (*linesw[tp->t_line].l_close)(tp, ap->a_fflag);
        ptsstop(tp, FREAD|FWRITE);
        (void) ttyclose(tp); /* clears t_state */
@@ -339,9 +341,13 @@ ptsclose(struct dev_close_args *ap)
                ptydebug(1, "master open? %s\n",
                    (pti->pt_flags2 & PF_MOPEN)?"yes":"no");
 
-               if (!(pti->pt_flags2 & PF_SOPEN) && !(pti->pt_flags2 & PF_MOPEN)) {
-                       devfs_clone_bitmap_put(&DEVFS_CLONE_BITMAP(pty), dev->si_uminor);
+               if (!(pti->pt_flags2 & PF_SOPEN) &&
+                   !(pti->pt_flags2 & PF_MOPEN)) {
                        destroy_dev(dev);
+                       devfs_clone_bitmap_put(&DEVFS_CLONE_BITMAP(pty),
+                                              uminor_no);
+                       ttyunregister(&pti->pt_tty);
+                       kfree(pti, M_PTY);
                }
        }
 #endif
@@ -503,8 +509,10 @@ ptcclose(struct dev_close_args *ap)
        cdev_t dev = ap->a_head.a_dev;
        struct tty *tp;
        struct pt_ioctl *pti = dev->si_drv1;
+       int uminor_no;
 
        tp = dev->si_tty;
+       uminor_no = dev->si_uminor;
        (void)(*linesw[tp->t_line].l_modem)(tp, 0);
 
 #ifdef UNIX98_PTYS
@@ -556,7 +564,10 @@ ptcclose(struct dev_close_args *ap)
                if (!(pti->pt_flags2 & PF_SOPEN)) {
                        ptydebug(1, "ptcclose: slaves are not open\n");
                        destroy_dev(pti->devs);
-                       devfs_clone_bitmap_put(&DEVFS_CLONE_BITMAP(pty), dev->si_uminor);
+                       devfs_clone_bitmap_put(&DEVFS_CLONE_BITMAP(pty),
+                                              uminor_no);
+                       ttyunregister(&pti->pt_tty);
+                       kfree(pti, M_PTY);
                }
        }
 #endif
index 4d0c993..85bce22 100644 (file)
@@ -115,7 +115,7 @@ struct tty {
        int     t_olowat;               /* Low water mark for output. */
        speed_t t_ospeedwat;            /* t_ospeed override for watermarks. */
        int     t_gen;                  /* Generation number. */
-       SLIST_ENTRY(tty) t_list;        /* Global chain of ttys for pstat(8) */
+       TAILQ_ENTRY(tty) t_list;        /* Global chain of ttys for pstat(8) */
 };
 
 #define        t_cc            t_termios.c_cc
@@ -185,6 +185,8 @@ struct tty {
 #define        TS_CTS_OFLOW    0x400000        /* For CCTS_OFLOW. */
 #define        TS_DSR_OFLOW    0x800000        /* For CDSR_OFLOW. */
 #endif
+#define TS_REGISTERED  0x1000000       /* ttyregister sanity check */
+#define TS_MARKER      0x2000000       /* sysctl iteration marker */
 
 /* Character type information. */
 #define        ORDINARY        0
@@ -274,7 +276,6 @@ int  ttyclose (struct tty *tp);
 void    ttyclearsession (struct tty *tp);
 void    ttyclosesession (struct session *, int);
 void    ttyflush (struct tty *tp, int rw);
-void    ttyfree (struct tty *tp);
 void    ttyinfo (struct tty *tp);
 int     ttyinput (int c, struct tty *tp);
 int     ttylclose (struct tty *tp, int flag);
@@ -284,6 +285,7 @@ int  ttyopen (cdev_t device, struct tty *tp);
 int     ttykqfilter (struct dev_kqfilter_args *);
 int     ttyread (struct dev_read_args *);
 void    ttyregister (struct tty *tp);
+void    ttyunregister (struct tty *tp);
 int     ttysleep (struct tty *tp, void *chan, int slpflags, char *wmesg,
            int timeout);
 int     ttyrevoke (struct dev_revoke_args *);