[src/trunk]: src/sys From Ilia Zykov:

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/7adbb61507c9
branches:  trunk
changeset: 803177:7adbb61507c9
user:      christos <christos%NetBSD.org@localhost>
date:      Wed Oct 15 15:00:03 2014 +0000

description:
>From Ilia Zykov:
- correct some incorrect comments
- add XXX warning
- increase security by activating when get the slave
- make pty_vn_open() private to tty_ptm.c

diffstat:

 sys/fs/ptyfs/ptyfs_subr.c   |  10 ++++++++--
 sys/fs/ptyfs/ptyfs_vfsops.c |  10 ++++++----
 sys/kern/tty_ptm.c          |  21 +++------------------
 sys/sys/pty.h               |   5 ++---
 4 files changed, 19 insertions(+), 27 deletions(-)

diffs (143 lines):

diff -r e8b4a031b484 -r 7adbb61507c9 sys/fs/ptyfs/ptyfs_subr.c
--- a/sys/fs/ptyfs/ptyfs_subr.c Wed Oct 15 14:54:25 2014 +0000
+++ b/sys/fs/ptyfs/ptyfs_subr.c Wed Oct 15 15:00:03 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ptyfs_subr.c,v 1.32 2014/08/16 07:22:30 hannken Exp $  */
+/*     $NetBSD: ptyfs_subr.c,v 1.33 2014/10/15 15:00:03 christos Exp $ */
 
 /*
  * Copyright (c) 1993
@@ -73,7 +73,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.32 2014/08/16 07:22:30 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.33 2014/10/15 15:00:03 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -177,6 +177,12 @@
            pp->ptyfs_atime = pp->ptyfs_ctime;
        pp->ptyfs_flags = 0;
        mutex_enter(&ptyfs_hashlock);
+       /*
+        * XXX We have minimum race condition when opening master side
+        * first time, if other threads through other mount points, trying
+        * opening the same device. As follow we have little chance have
+        * unused list entries.
+        */
        SLIST_INSERT_HEAD(ppp, pp, ptyfs_hash);
        mutex_exit(&ptyfs_hashlock);
        return pp;
diff -r e8b4a031b484 -r 7adbb61507c9 sys/fs/ptyfs/ptyfs_vfsops.c
--- a/sys/fs/ptyfs/ptyfs_vfsops.c       Wed Oct 15 14:54:25 2014 +0000
+++ b/sys/fs/ptyfs/ptyfs_vfsops.c       Wed Oct 15 15:00:03 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ptyfs_vfsops.c,v 1.53 2014/08/15 13:40:39 hannken Exp $        */
+/*     $NetBSD: ptyfs_vfsops.c,v 1.54 2014/10/15 15:00:03 christos Exp $       */
 
 /*
  * Copyright (c) 1992, 1993, 1995
@@ -38,7 +38,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ptyfs_vfsops.c,v 1.53 2014/08/15 13:40:39 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ptyfs_vfsops.c,v 1.54 2014/10/15 15:00:03 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -217,7 +217,8 @@
                *vpp = NULL;
                return error;
        }
-       if (type == PTYFSptc)
+       /* Activate node only after we have grabbed device. */
+       if (type == PTYFSpts)
                ptyfs_set_active(mp, minor(dev));
        return 0;
 }
@@ -415,7 +416,8 @@
 
 /*
  * Initialize this vnode / ptynode pair.
- * Caller assures no other thread will try to load this node.
+ * Only for the slave side of a pty, caller assures
+ * no other thread will try to load this node.
  */
 int
 ptyfs_loadvnode(struct mount *mp, struct vnode *vp,
diff -r e8b4a031b484 -r 7adbb61507c9 sys/kern/tty_ptm.c
--- a/sys/kern/tty_ptm.c        Wed Oct 15 14:54:25 2014 +0000
+++ b/sys/kern/tty_ptm.c        Wed Oct 15 15:00:03 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tty_ptm.c,v 1.34 2014/09/05 09:20:59 matt Exp $        */
+/*     $NetBSD: tty_ptm.c,v 1.35 2014/10/15 15:00:03 christos Exp $    */
 
 /*-
  * Copyright (c) 2004 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tty_ptm.c,v 1.34 2014/09/05 09:20:59 matt Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tty_ptm.c,v 1.35 2014/10/15 15:00:03 christos Exp $");
 
 #include "opt_compat_netbsd.h"
 #include "opt_ptm.h"
@@ -87,6 +87,7 @@
 static dev_t pty_getfree(void);
 static int pty_alloc_master(struct lwp *, int *, dev_t *, struct mount *);
 static int pty_alloc_slave(struct lwp *, int *, dev_t, struct mount *);
+static int pty_vn_open(struct vnode *, struct lwp *);
 
 void ptmattach(int);
 
@@ -174,22 +175,6 @@
                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;
diff -r e8b4a031b484 -r 7adbb61507c9 sys/sys/pty.h
--- a/sys/sys/pty.h     Wed Oct 15 14:54:25 2014 +0000
+++ b/sys/sys/pty.h     Wed Oct 15 15:00:03 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pty.h,v 1.10 2014/04/04 18:11:58 christos Exp $        */
+/*     $NetBSD: pty.h,v 1.11 2014/10/15 15:00:03 christos Exp $        */
 
 /*-
  * Copyright (c) 2004 The NetBSD Foundation, Inc.
@@ -39,14 +39,13 @@
 int pty_fill_ptmget(struct lwp *, dev_t, int, int, void *, struct mount *);
 int pty_grant_slave(struct lwp *, dev_t, struct mount *);
 dev_t pty_makedev(char, int);
-int pty_vn_open(struct vnode *, struct lwp *);
 struct ptm_pty *pty_sethandler(struct ptm_pty *);
 int pty_getmp(struct lwp *, struct mount **);
 
 /*
  * Ptm_pty is used for switch ptm{x} driver between BSDPTY, PTYFS.
  * Functions' argument (struct mount *) is used only PTYFS,
- * in the case BSDPTY can be NULL, and arg must be NULL.
+ * in the case BSDPTY can be NULL.
  */
 struct ptm_pty {
        int (*allocvp)(struct mount *, struct lwp *, struct vnode **, dev_t,