From: Matthew Dillon <dillon@apollo.backplane.com>
Date: Fri, 13 Aug 2010 01:55:20 +0000 (-0700)
Subject: kernel - Fix pty registration, memory leak
X-Git-Tag: v2.8.0~554
X-Git-Url: http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/ddac2002e1c0b5c573dc3e05d7d2ebbab9547af6

kernel - Fix pty registration, memory leak

* 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.
---

diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index 6c24a89..5c2a701 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -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,
diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c
index b21907f..ddf0287 100644
--- a/sys/kern/tty_pty.c
+++ b/sys/kern/tty_pty.c
@@ -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
diff --git a/sys/sys/tty.h b/sys/sys/tty.h
index 4d0c993..85bce22 100644
--- a/sys/sys/tty.h
+++ b/sys/sys/tty.h
@@ -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 *);