tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Enhance ptyfs to handle multiple instances.



On Apr 2, 10:36am, netbsd%izyk.ru@localhost (Ilya Zykov) wrote:
-- Subject: Re: Enhance ptyfs to handle multiple instances.

Looks very good. Some changes:

- I don't like the refactoring because it makes ptyfs less optional (brings
  in code and headers to the base kernel). I think it is simpler to provide
  an entry function to get the mount point instead, and this way all the guts
  of ptyfs stay in ptyfs.
- Is it important to append to the list? Then perhaps use a different set
  of macros than LIST_. I've changed the code just to prepend.

I hope I did not break it. Comments?

christos

Index: sys/pty.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pty.h,v
retrieving revision 1.9
diff -u -p -u -r1.9 pty.h
--- sys/pty.h   27 Mar 2014 17:31:56 -0000      1.9
+++ sys/pty.h   3 Apr 2014 21:51:03 -0000
@@ -41,7 +41,7 @@ int pty_grant_slave(struct lwp *, dev_t,
 dev_t pty_makedev(char, int);
 int pty_vn_open(struct vnode *, struct lwp *);
 struct ptm_pty *pty_sethandler(struct ptm_pty *);
-int ptyfs_getmp(struct lwp *, struct mount **);
+int pty_getmp(struct lwp *, struct mount **);
 
 /*
  * Ptm_pty is used for switch ptm{x} driver between BSDPTY, PTYFS.
@@ -53,7 +53,7 @@ struct ptm_pty {
            char);
        int (*makename)(struct mount *, struct lwp *, char *, size_t, dev_t, 
char);
        void (*getvattr)(struct mount *, struct lwp *, struct vattr *);
-       void *arg;
+       int (*getmp)(struct lwp *, struct mount **);
 };
 
 #ifdef COMPAT_BSDPTY
Index: fs/ptyfs/ptyfs.h
===================================================================
RCS file: /cvsroot/src/sys/fs/ptyfs/ptyfs.h,v
retrieving revision 1.11
diff -u -p -u -r1.11 ptyfs.h
--- fs/ptyfs/ptyfs.h    21 Mar 2014 17:21:53 -0000      1.11
+++ fs/ptyfs/ptyfs.h    3 Apr 2014 21:51:04 -0000
@@ -106,6 +106,8 @@ struct ptyfsnode {
 };
 
 struct ptyfsmount {
+       LIST_ENTRY(ptyfsmount) pmnt_le;
+       struct mount *pmnt_mp;
        gid_t pmnt_gid;
        mode_t pmnt_mode;
        int pmnt_flags;
Index: fs/ptyfs/ptyfs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.48
diff -u -p -u -r1.48 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c     27 Mar 2014 17:31:56 -0000      1.48
+++ fs/ptyfs/ptyfs_vfsops.c     3 Apr 2014 21:51:04 -0000
@@ -77,6 +77,7 @@ static int ptyfs__allocvp(struct mount *
 static int ptyfs__makename(struct mount *, struct lwp *, char *, size_t,
     dev_t, char);
 static void ptyfs__getvattr(struct mount *, struct lwp *, struct vattr *);
+static int ptyfs__getmp(struct lwp *, struct mount **);
 
 /*
  * ptm glue: When we mount, we make ptm point to us.
@@ -84,13 +85,37 @@ static void ptyfs__getvattr(struct mount
 struct ptm_pty *ptyfs_save_ptm;
 static int ptyfs_count;
 
+static LIST_HEAD(, ptyfsmount) ptyfs_head;
+
 struct ptm_pty ptm_ptyfspty = {
        ptyfs__allocvp,
        ptyfs__makename,
        ptyfs__getvattr,
-       NULL
+       ptyfs__getmp,
 };
 
+static int
+ptyfs__getmp(struct lwp *l, struct mount **mpp)
+{
+       struct cwdinfo *cwdi = l->l_proc->p_cwdi;
+       struct mount *mp;
+       struct ptyfsmount *pmnt;
+ 
+       LIST_FOREACH(pmnt, &ptyfs_head, pmnt_le) {
+               mp = pmnt->pmnt_mp;
+               if (cwdi->cwdi_rdir == NULL)
+                       goto ok;
+
+               if (vn_isunder(mp->mnt_vnodecovered, cwdi->cwdi_rdir, l))
+                       goto ok;
+       }
+       *mpp = NULL;
+       return EOPNOTSUPP;
+ok:
+       *mpp = mp;
+       return 0;
+}
+
 static const char *
 ptyfs__getpath(struct lwp *l, const struct mount *mp)
 {
@@ -137,6 +162,18 @@ ptyfs__makename(struct mount *mp, struct
                len = snprintf(tbuf, bufsiz, "/dev/null");
                break;
        case 't':
+               /*
+                * We support traditional ptys, so we can get here,
+                * if pty had been opened before PTYFS was mounted,
+                * or was opened through /dev/ptyXX devices.
+                * Return it only outside chroot for more security :).
+                */
+               if (l->l_proc->p_cwdi->cwdi_rdir == NULL
+                   && ptyfs_save_ptm != NULL 
+                   && ptyfs_used_get(PTYFSptc, minor(dev), mp, 0) == NULL)
+                       return (*ptyfs_save_ptm->makename)(mp, l,
+                           tbuf, bufsiz, dev, ms);
+
                np = ptyfs__getpath(l, mp);
                if (np == NULL)
                        return EOPNOTSUPP;
@@ -189,6 +226,7 @@ void
 ptyfs_init(void)
 {
 
+       LIST_INIT(&ptyfs_head);
        malloc_type_attach(M_PTYFSMNT);
        malloc_type_attach(M_PTYFSTMP);
        ptyfs_hashinit();
@@ -274,9 +312,9 @@ ptyfs_mount(struct mount *mp, const char
                return error;
        }
 
-       /* Point pty access to us */
-       if (ptyfs_count == 0) {
-               ptm_ptyfspty.arg = mp;
+       LIST_INSERT_HEAD(&ptyfs_head, pmnt, pmnt_le);
+       if (ptyfs_count++ == 0) {
+               /* Point pty access to us */
                ptyfs_save_ptm = pty_sethandler(&ptm_ptyfspty);
        }
        ptyfs_count++;
@@ -296,6 +334,7 @@ ptyfs_unmount(struct mount *mp, int mntf
 {
        int error;
        int flags = 0;
+       struct ptyfsmount *pmnt;
 
        if (mntflags & MNT_FORCE)
                flags |= FORCECLOSE;
@@ -308,8 +347,13 @@ ptyfs_unmount(struct mount *mp, int mntf
                /* Restore where pty access was pointing */
                (void)pty_sethandler(ptyfs_save_ptm);
                ptyfs_save_ptm = NULL;
-               ptm_ptyfspty.arg = NULL;
        }
+       LIST_FOREACH(pmnt, &ptyfs_head, pmnt_le) {
+               if (pmnt->pmnt_mp == mp) {
+                       LIST_REMOVE(pmnt, pmnt_le);
+                       break;
+               }
+       }
 
        /*
         * Finally, throw away the ptyfsmount structure
Index: fs/ptyfs/ptyfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c      27 Mar 2014 21:13:06 -0000      1.45
+++ fs/ptyfs/ptyfs_vnops.c      3 Apr 2014 21:51:04 -0000
@@ -141,6 +141,7 @@ int ptyfs_readdir   (void *);
 #define        ptyfs_readlink  genfs_eopnotsupp
 #define        ptyfs_abortop   genfs_abortop
 int    ptyfs_reclaim   (void *);
+int    ptyfs_inactive  (void *);
 #define        ptyfs_lock      genfs_lock
 #define        ptyfs_unlock    genfs_unlock
 #define        ptyfs_bmap      genfs_badop
@@ -192,7 +193,7 @@ const struct vnodeopv_entry_desc ptyfs_v
        { &vop_readdir_desc, ptyfs_readdir },           /* readdir */
        { &vop_readlink_desc, ptyfs_readlink },         /* readlink */
        { &vop_abortop_desc, ptyfs_abortop },           /* abortop */
-       { &vop_inactive_desc, spec_inactive },          /* inactive */
+       { &vop_inactive_desc, ptyfs_inactive },         /* inactive */
        { &vop_reclaim_desc, ptyfs_reclaim },           /* reclaim */
        { &vop_lock_desc, ptyfs_lock },                 /* lock */
        { &vop_unlock_desc, ptyfs_unlock },             /* unlock */
@@ -225,6 +226,28 @@ ptyfs_reclaim(void *v)
        return ptyfs_freevp(ap->a_vp);
 }
 
+int
+ptyfs_inactive(void *v)
+{
+       struct vop_inactive_args /* {
+               struct vnode *a_vp;
+               bool *a_recycle;
+       } */ *ap = v;
+       struct vnode *vp = ap->a_vp;
+       struct ptyfsnode *ptyfs = VTOPTYFS(vp);
+
+       switch (ptyfs->ptyfs_type) {
+       case PTYFSpts:
+       case PTYFSptc:
+               /* Emulate file deletion for call reclaim(). */
+               *ap->a_recycle = true;
+               break;
+       default:
+               break;
+       }
+       return spec_inactive(v);
+}
+
 /*
  * Return POSIX pathconf information applicable to special devices.
  */
Index: kern/tty_bsdpty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty_bsdpty.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 tty_bsdpty.c
--- kern/tty_bsdpty.c   27 Mar 2014 17:31:56 -0000      1.19
+++ kern/tty_bsdpty.c   3 Apr 2014 21:51:04 -0000
@@ -74,12 +74,13 @@ static int pty_makename(struct mount *, 
 static int pty_allocvp(struct mount *, struct lwp *, struct vnode **,
     dev_t, char);
 static void pty_getvattr(struct mount *, struct lwp *, struct vattr *);
+static intl pty_getmp(struct lwp *, struct mount **);
 
 struct ptm_pty ptm_bsdpty = {
        pty_allocvp,
        pty_makename,
        pty_getvattr,
-       NULL
+       pty_getmp,
 };
 
 static int
@@ -152,5 +153,13 @@ pty_getvattr(struct mount *mp, struct lw
        vattr->va_gid = TTY_GID;
        vattr->va_mode = TTY_PERM;
 }
+
+static int
+pty_getmp(struct lwp *l __unused, struct mount **mpp)
+{
+       *mpp = 0;
+       return 0;
+}
+
 #endif /* COMPAT_BSDPTY */
 #endif /* NO_DEV_PTM */
Index: kern/tty_ptm.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.31
diff -u -p -u -r1.31 tty_ptm.c
--- kern/tty_ptm.c      27 Mar 2014 17:31:56 -0000      1.31
+++ kern/tty_ptm.c      3 Apr 2014 21:51:04 -0000
@@ -90,31 +90,12 @@ static int pty_alloc_slave(struct lwp *,
 void ptmattach(int);
 
 int
-ptyfs_getmp(struct lwp *l, struct mount **mpp) {
-       struct cwdinfo *cwdi = l->l_proc->p_cwdi;
-       struct mount *mp;
-
+pty_getmp(struct lwp *l, struct mount **mpp)
+{
        if (ptm == NULL)
                return EOPNOTSUPP;
 
-       if (ptm->arg == NULL) { /* BSDPTY */
-               *mpp = NULL;
-               return 0;
-       }
-
-       mp = ptm->arg;  /* PTYFS */
-
-       if (cwdi->cwdi_rdir == NULL)
-               goto ok;
-
-       if (vn_isunder(mp->mnt_vnodecovered, cwdi->cwdi_rdir, l))
-               goto ok;
-
-       *mpp = NULL;
-       return EOPNOTSUPP;
-ok:
-       *mpp = mp;
-       return 0;
+       return (*ptm->getmp)(l, mpp);
 }
 
 dev_t
@@ -192,6 +173,22 @@ 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;
@@ -355,7 +352,7 @@ ptmopen(dev_t dev, int flag, int mode, s
        switch(minor(dev)) {
        case 0:         /* /dev/ptmx */
        case 2:         /* /emul/linux/dev/ptmx */
-               if ((error = ptyfs_getmp(l, &mp)) != 0)
+               if ((error = pty_getmp(l, &mp)) != 0)
                        return error;
                if ((error = pty_alloc_master(l, &fd, &ttydev, mp)) != 0)
                        return error;
@@ -403,7 +400,7 @@ ptmioctl(dev_t dev, u_long cmd, void *da
        error = 0;
        switch (cmd) {
        case TIOCPTMGET:
-               if ((error = ptyfs_getmp(l, &mp)) != 0)
+               if ((error = pty_getmp(l, &mp)) != 0)
                        return error;
 
                if ((error = pty_alloc_master(l, &cfd, &newdev, mp)) != 0)
Index: kern/tty_pty.c
===================================================================
RCS file: /cvsroot/src/sys/kern/tty_pty.c,v
retrieving revision 1.137
diff -u -p -u -r1.137 tty_pty.c
--- kern/tty_pty.c      28 Mar 2014 11:55:09 -0000      1.137
+++ kern/tty_pty.c      3 Apr 2014 21:51:04 -0000
@@ -1075,7 +1075,7 @@ ptyioctl(dev_t dev, u_long cmd, void *da
 #ifndef NO_DEV_PTM
        /* Allow getting the name from either the master or the slave */
        if (cmd == TIOCPTSNAME) {
-               if ((error = ptyfs_getmp(l, &mp)) != 0)
+               if ((error = pty_getmp(l, &mp)) != 0)
                        return error;
                return pty_fill_ptmget(l, dev, -1, -1, data, mp);
        }
@@ -1086,7 +1086,7 @@ ptyioctl(dev_t dev, u_long cmd, void *da
                switch (cmd) {
 #ifndef NO_DEV_PTM
                case TIOCGRANTPT:
-                       if ((error = ptyfs_getmp(l, &mp)) != 0)
+                       if ((error = pty_getmp(l, &mp)) != 0)
                                return error;
                        return pty_grant_slave(l, dev, mp);
 #endif


Home | Main Index | Thread Index | Old Index