Re: Ptyfs correction.

[Ilia Zykov <netbsd%izyk.ru@localhost>]

Sorry, forgot free().
Ilia.

Index: fs/ptyfs/ptyfs_subr.c
===================================================================
RCS file: /cvsroot/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.29.4.1
diff -u -p -r1.29.4.1 ptyfs_subr.c
--- fs/ptyfs/ptyfs_subr.c	17 Aug 2014 03:34:02 -0000	1.29.4.1
+++ fs/ptyfs/ptyfs_subr.c	13 Oct 2014 11:48:40 -0000
@@ -97,6 +97,9 @@ static kmutex_t ptyfs_hashlock;
 
 static SLIST_HEAD(ptyfs_hashhead, ptyfsnode) *ptyfs_node_tbl;
 static u_long ptyfs_node_mask; /* size of hash table - 1 */
+static struct ptyfsnode *
+ptyfs_lookup_node(ptyfstype, int, struct ptyfs_hashhead *);
+
 
 /*
  * allocate a ptyfsnode/vnode pair.  the vnode is referenced.
@@ -140,24 +143,36 @@ ptyfs_hashdone(void)
 }
 
 /*
- * Get a ptyfsnode from the hash table, or allocate one.
+ * Lookup a ptyfsnode in the hash table.
  */
-struct ptyfsnode *
-ptyfs_get_node(ptyfstype type, int pty)
+static struct ptyfsnode *
+ptyfs_lookup_node(ptyfstype type, int pty, struct ptyfs_hashhead *ppp)
 {
-	struct ptyfs_hashhead *ppp;
 	struct ptyfsnode *pp;
-
-	ppp = &ptyfs_node_tbl[PTYFS_FILENO(type, pty) & ptyfs_node_mask];
-
-	mutex_enter(&ptyfs_hashlock);
+	/*
+	 * We can use this list without lock, because we use only
+	 * SLIST_INSERT_HEAD for insert an element, and never
+	 * use remove macros.
+	 */
 	SLIST_FOREACH(pp, ppp, ptyfs_hash) {
 		if (pty == pp->ptyfs_pty && pp->ptyfs_type == type) {
-			mutex_exit(&ptyfs_hashlock);
 			return pp;
 		}
 	}
-	mutex_exit(&ptyfs_hashlock);
+	return NULL;
+}
+/*
+ * Get a ptyfsnode from the hash table, or allocate one.
+ */
+struct ptyfsnode *
+ptyfs_get_node(ptyfstype type, int pty)
+{
+	struct ptyfs_hashhead *ppp;
+	struct ptyfsnode *pp, *tpp;
+
+	ppp = &ptyfs_node_tbl[PTYFS_FILENO(type, pty) & ptyfs_node_mask];
+	if((pp = ptyfs_lookup_node(type, pty, ppp)) != NULL)
+		return pp;
 
 	pp = malloc(sizeof(struct ptyfsnode), M_TEMP, M_WAITOK);
 	pp->ptyfs_pty = pty;
@@ -177,8 +192,20 @@ ptyfs_get_node(ptyfstype type, int pty)
 	    pp->ptyfs_atime = pp->ptyfs_ctime;
 	pp->ptyfs_flags = 0;
 	mutex_enter(&ptyfs_hashlock);
-	SLIST_INSERT_HEAD(ppp, pp, ptyfs_hash);
+	/*
+	 * We can have the race condition than open master side. Because
+	 * caller can't assures no other thread will try to load this node,
+	 * if we have multiple mount points.
+	 */
+	if (type != PTYFSptc ||
+		(tpp = ptyfs_lookup_node(type, pty, ppp)) == NULL) {
+			SLIST_INSERT_HEAD(ppp, pp, ptyfs_hash);
+	}
 	mutex_exit(&ptyfs_hashlock);
+	if (tpp) {
+		free(pp, M_TEMP);
+		return tpp;
+	}
 	return pp;
 }
 
Index: fs/ptyfs/ptyfs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.50.2.1
diff -u -p -r1.50.2.1 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c	17 Aug 2014 03:34:02 -0000	1.50.2.1
+++ fs/ptyfs/ptyfs_vfsops.c	13 Oct 2014 11:48:40 -0000
@@ -416,6 +416,7 @@ ptyfs_sync(struct mount *mp, int waitfor
 /*
  * Initialize this vnode / ptynode pair.
  * Caller assures no other thread will try to load this node.
+ * Only for the slave side of a pty.
  */
 int
 ptyfs_loadvnode(struct mount *mp, struct vnode *vp,
Index: kern/tty_ptm.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.33
diff -u -p -r1.33 tty_ptm.c
--- kern/tty_ptm.c	25 Jul 2014 08:10:40 -0000	1.33
+++ kern/tty_ptm.c	13 Oct 2014 11:48:40 -0000
@@ -174,22 +174,6 @@ retry:
 		error = EOPNOTSUPP;
 		goto bad;
 	}
-	/*
-	 * XXX Since PTYFS has now multiple instance support, if we mounted
-	 * more than one PTYFS we must check here the ptyfs_used_tbl, to find
-	 * out if the ptyfsnode is under the appropriate mount and skip the
-	 * node if not, because the pty could has been released, but
-	 * ptyfs_reclaim didn't get a chance to release the corresponding
-	 * node other mount point yet.
-	 *
-	 * It's important to have only one mount point's ptyfsnode for each
-	 * appropriate device in ptyfs_used_tbl, else we will have a security 
-	 * problem, because every entry will have access to this device.
-	 *
-	 * Also we will not have not efficient vnode and memory usage.
-	 * You can test this by changing a_recycle from true to false
-	 * in ptyfs_inactive.
-	 */
 	if ((error = (*ptm->allocvp)(mp, l, &vp, *dev, 'p')) != 0) {
 		DPRINTF(("pty_allocvp %d\n", error));
 		goto bad;
@@ -197,6 +181,8 @@ retry:
 
 	if ((error = pty_vn_open(vp, l)) != 0) {
 		DPRINTF(("pty_vn_open %d\n", error));
+		VOP_UNLOCK(vp);
+		vrele(vp);
 		/*
 		 * Check if the master open failed because we lost
 		 * the race to grab it.