Re: Enhance ptyfs to handle multiple instances.
- 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. Looks better, thank you. - 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? Order IMPORTANT here, because, pty_getmp returns the first found, traditionally /dev/pts - must be persistently first(for security too). All others MPs are useful only inside chroot. Ilya.
Re: Enhance ptyfs to handle multiple instances.
Some misspelling corrections.
fs/ptyfs/ptyfs.h|2 +
fs/ptyfs/ptyfs_vfsops.c | 63 ++--
fs/ptyfs/ptyfs_vnops.c | 25 ++-
kern/tty_bsdpty.c | 11 +++-
kern/tty_ptm.c | 45 --
kern/tty_pty.c |4 +--
sys/pty.h |4 +--
7 files changed, 117 insertions(+), 37 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs.h
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs.h,v
retrieving revision 1.6
diff -u -p -r1.6 ptyfs.h
--- fs/ptyfs/ptyfs.h 24 Mar 2014 20:48:08 - 1.6
+++ fs/ptyfs/ptyfs.h 4 Apr 2014 10:37:57 -
@@ -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: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.13
diff -u -p -r1.13 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 28 Mar 2014 10:53:57 - 1.13
+++ fs/ptyfs/ptyfs_vfsops.c 4 Apr 2014 10:37:57 -
@@ -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();
@@ -218,7 +256,7 @@ ptyfs_mount(struct mount *mp, const char
{
struct lwp *l = curlwp;
int error = 0;
- struct ptyfsmount *pmnt;
+ struct ptyfsmount *pmnt, *pmnt_next;
struct ptyfs_args *args = data;
if (*data_len != sizeof *args *data_len != OSIZE)
@@ -274,12 +312,17 @@ ptyfs_mount(struct mount *mp, const char
return error;
}
- /* Point pty access to us */
- if (ptyfs_count == 0) {
- ptm_ptyfspty.arg = mp;
+ pmnt-pmnt_mp = mp;
+ if (ptyfs_count++ == 0) {
+ LIST_INSERT_HEAD(ptyfs_head, pmnt, pmnt_le);
+ /* Point pty access to us */
ptyfs_save_ptm = pty_sethandler(ptm_ptyfspty);
+ } else {
+ pmnt_next = LIST_FIRST(ptyfs_head);
+ while(LIST_NEXT(pmnt_next, pmnt_le) != NULL)
+ pmnt_next = LIST_NEXT(pmnt_next, pmnt_le);
+ LIST_INSERT_AFTER(pmnt_next, pmnt, pmnt_le);
}
- ptyfs_count++;
return 0;
}
@@ -296,6 +339,7 @@ ptyfs_unmount(struct mount *mp, int mntf
{
int error;
int flags = 0;
+ struct ptyfsmount *pmnt;
if (mntflags MNT_FORCE)
flags |= FORCECLOSE;
@@ -308,8 +352,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: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.5
diff -u -p -r1.5 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c 28 Mar 2014 10:53:58 - 1.5
+++ fs/ptyfs/ptyfs_vnops.c 4 Apr 2014 10:37:57 -
@@ -141,6 +141,7 @@ int ptyfs_readdir (void *);
#define ptyfs_readlink genfs_eopnotsupp
#define
Re: Enhance ptyfs to handle multiple instances.
On Apr 4, 12:29pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | - 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. | | Looks better, thank you. | | - 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? | | Order IMPORTANT here, because, pty_getmp returns the first found, | traditionally /dev/pts - must be persistently first(for security too). | All others MPs are useful only inside chroot. Should we put a pointer in the pty node that points to the primary mount point then so we get the correct one? Or that does not work? I will change the list so it always appends. Thanks, christos
Re: Enhance ptyfs to handle multiple instances.
Should we put a pointer in the pty node that points to the primary mount point then so we get the correct one? Why? In general case we forever must return first which mount first, next mount point, shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. Simple appends it to the tail will be good(IMHO). Or that does not work? I will change the list so it always appends. Ilya.
Re: Enhance ptyfs to handle multiple instances.
On 04.04.2014 18:40, Ilya Zykov wrote: Should we put a pointer in the pty node that points to the primary mount point then so we get the correct one? Why? In general case we forever must return first which mount first, next mount point, shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. Simple appends it to the tail will be good(IMHO). Sorry, TIOCPTSNAME of course.
Re: Enhance ptyfs to handle multiple instances.
On Apr 4, 6:40pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | Should we put a pointer in the pty node that points to the primary mount point | then so we get the correct one? | | Why? In general case we forever must return first which mount first, next mount point, | shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. | Simple appends it to the tail will be good(IMHO). I have to think about it. If you opened a pty in a chroot, the pty node will appear both in the chroot and in the regular mount. If you opened a pty outside the chroot, the pty will appear only in the regular mount and not in the chroot, right? If you have 2 chroots, each one will only show its own ptys, but the regular not rooted mount will show all of them? | Or that does not work? I will change the list | so it always appends. I'll convert to an STAILQ christos
Re: Enhance ptyfs to handle multiple instances.
On 04.04.2014 18:55, Christos Zoulas wrote: On Apr 4, 6:40pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | Should we put a pointer in the pty node that points to the primary mount point | then so we get the correct one? | | Why? In general case we forever must return first which mount first, next mount point, | shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. | Simple appends it to the tail will be good(IMHO). I have to think about it. If you opened a pty in a chroot, the pty node will appear both in the chroot and in the regular mount. If you opened a pty outside the chroot, the pty will appear only in the regular mount and not in the chroot, right? If you have 2 chroots, each one will only show its own ptys, but the regular not rooted mount will show all of them? Yes.
Re: Enhance ptyfs to handle multiple instances.
On 04.04.2014 18:55, Christos Zoulas wrote: On Apr 4, 6:40pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | Should we put a pointer in the pty node that points to the primary mount point | then so we get the correct one? | | Why? In general case we forever must return first which mount first, next mount point, | shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. | Simple appends it to the tail will be good(IMHO). I have to think about it. If you opened a pty in a chroot, the pty node will appear both in the chroot and in the regular mount. If you opened a pty outside the chroot, the pty will appear only in the regular mount and not in the chroot, right? If you have 2 chroots, each one will only show its own ptys, but the regular not rooted mount will show all of them? Yes. But every is in proper MP.
Re: Enhance ptyfs to handle multiple instances.
On 04.04.2014 18:55, Christos Zoulas wrote: On Apr 4, 6:40pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | Should we put a pointer in the pty node that points to the primary mount point | then so we get the correct one? | | Why? In general case we forever must return first which mount first, next mount point, | shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. | Simple appends it to the tail will be good(IMHO). I have to think about it. If you opened a pty in a chroot, the pty node will appear both in the chroot and in the regular mount. If you opened a pty outside the chroot, the pty will appear only in the regular mount and not in the chroot, right? If you have 2 chroots, each one will only show its own ptys, but the regular not rooted mount will show all of them? Maybe I do not understand you correctly. No. Every is in proper MP.
Re: Enhance ptyfs to handle multiple instances.
On Apr 4, 7:28pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | On 04.04.2014 18:55, Christos Zoulas wrote: | On Apr 4, 6:40pm, net...@izyk.ru (Ilya Zykov) wrote: | -- Subject: Re: Enhance ptyfs to handle multiple instances. | | | | | Should we put a pointer in the pty node that points to the primary mount point | | then so we get the correct one? | | | | Why? In general case we forever must return first which mount first, next mount point, | | shouldn't replace previous, else incorrect TIOCPTMGET(path) for already opened pty we will have. | | Simple appends it to the tail will be good(IMHO). | | I have to think about it. If you opened a pty in a chroot, the pty node | will appear both in the chroot and in the regular mount. If you opened | a pty outside the chroot, the pty will appear only in the regular mount | and not in the chroot, right? If you have 2 chroots, each one will only | show its own ptys, but the regular not rooted mount will show all of them? | | | Maybe I do not understand you correctly. | No. Every is in proper MP. I'll test it. christos
Re: Enhance ptyfs to handle multiple instances.
Small test.
#include util.h
#include stdio.h
#include termios.h
#include err.h
#include stdlib.h
#include sys/types.h
#include sys/ioctl.h
#include sys/stat.h
#include errno.h
#include fcntl.h
#include grp.h
#include stdio.h
#include string.h
#include unistd.h
main (){
int amaster, aslave, master;
char name[100] = /dev/ptm;
struct termios termp;
struct winsize winp;
printf(Pty device is : %s \n, name);
//if ( openpty(amaster, aslave, name, termp, winp) == -1 )
if ((master = open(name, O_RDWR)) != -1) {
struct ptmget pt;
// if (ioctl(master, TIOCPTSNAME, pt) != -1) {
if (ioctl(master, TIOCPTMGET, pt) != -1) {
//(void)close(master);
master = pt.cfd;
aslave = pt.sfd;
(void)strcpy(name, pt.sn);
goto gotit;
}
err(EXIT_FAILURE, Can't ioctl() TIOCPTSNAME);
}
err(EXIT_FAILURE, Can't open /dev/ptmx);
gotit:
printf(Name is : %s \n, name);
open(name, O_RDWR);
sleep(60);
}
Re: Enhance ptyfs to handle multiple instances.
On Apr 2, 10:36am, net...@izyk.ru (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 - 1.9
+++ sys/pty.h 3 Apr 2014 21:51:03 -
@@ -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.h21 Mar 2014 17:21:53 - 1.11
+++ fs/ptyfs/ptyfs.h3 Apr 2014 21:51:04 -
@@ -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 - 1.48
+++ fs/ptyfs/ptyfs_vfsops.c 3 Apr 2014 21:51:04 -
@@ -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
Re: Enhance ptyfs to handle multiple instances.
On 01.04.2014 23:38, Ilya Zykov wrote:
Hello!
This patch introduces subject.
Some code refactoring, because we have more stronger ptm - ptyfs binding.
BSD pty compatibility improvement.
Main explanation you can see in comments inside.
Also I am not sure about, how, correctly release unused vnode and return it
for system and call reclaim. Before anybody call for it getnewvnode.
fs/ptyfs/ptyfs.h| 21 +
fs/ptyfs/ptyfs_subr.c |2 --
fs/ptyfs/ptyfs_vfsops.c | 37 +
fs/ptyfs/ptyfs_vnops.c | 26 --
kern/tty_bsdpty.c |4 ++--
kern/tty_ptm.c | 31 +++
sys/pty.h | 21 -
7 files changed, 99 insertions(+), 43 deletions(-)
Ilya.
XXX comments change.
fs/ptyfs/ptyfs.h| 21 +
fs/ptyfs/ptyfs_subr.c |2 --
fs/ptyfs/ptyfs_vfsops.c | 37 +
fs/ptyfs/ptyfs_vnops.c | 26 --
kern/tty_bsdpty.c |4 ++--
kern/tty_ptm.c | 30 ++
sys/pty.h | 21 -
7 files changed, 98 insertions(+), 43 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs.h
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs.h,v
retrieving revision 1.6
diff -u -p -r1.6 ptyfs.h
--- fs/ptyfs/ptyfs.h 24 Mar 2014 20:48:08 - 1.6
+++ fs/ptyfs/ptyfs.h 2 Apr 2014 06:28:40 -
@@ -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;
@@ -113,6 +115,25 @@ struct ptyfsmount {
#define VFSTOPTY(mp) ((struct ptyfsmount *)(mp)-mnt_data)
+LIST_HEAD(ptyfs_lhmp, ptyfsmount);
+
+/*
+ * 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.
+ */
+struct ptm_pty {
+ int (*allocvp)(struct mount *, struct lwp *, struct vnode **, dev_t,
+ char);
+ int (*makename)(struct mount *, struct lwp *, char *, size_t, dev_t, char);
+ void (*getvattr)(struct mount *, struct lwp *, struct vattr *);
+ struct ptyfs_lhmp *arg;
+};
+
+dev_t pty_makedev(char, int);
+int pty_vn_open(struct vnode *, struct lwp *);
+struct ptm_pty *pty_sethandler(struct ptm_pty *);
+
#endif /* _KERNEL */
struct ptyfs_args {
Index: fs/ptyfs/ptyfs_subr.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.6
diff -u -p -r1.6 ptyfs_subr.c
--- fs/ptyfs/ptyfs_subr.c 28 Mar 2014 10:53:57 - 1.6
+++ fs/ptyfs/ptyfs_subr.c 2 Apr 2014 06:28:40 -
@@ -86,8 +86,6 @@ __KERNEL_RCSID(0, $NetBSD: ptyfs_subr.c
#include sys/namei.h
#include sys/filedesc.h
#include sys/select.h
-#include sys/tty.h
-#include sys/pty.h
#include sys/kauth.h
#include sys/lwp.h
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.13
diff -u -p -r1.13 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 28 Mar 2014 10:53:57 - 1.13
+++ fs/ptyfs/ptyfs_vfsops.c 2 Apr 2014 06:28:40 -
@@ -54,8 +54,6 @@ __KERNEL_RCSID(0, $NetBSD: ptyfs_vfsops
#include sys/syslog.h
#include sys/select.h
#include sys/filedesc.h
-#include sys/tty.h
-#include sys/pty.h
#include sys/kauth.h
#include sys/module.h
@@ -83,12 +81,13 @@ static void ptyfs__getvattr(struct mount
*/
struct ptm_pty *ptyfs_save_ptm;
static int ptyfs_count;
+static struct ptyfs_lhmp mph;
struct ptm_pty ptm_ptyfspty = {
ptyfs__allocvp,
ptyfs__makename,
ptyfs__getvattr,
- NULL
+ mph
};
static const char *
@@ -130,6 +129,7 @@ ptyfs__makename(struct mount *mp, struct
{
size_t len;
const char *np;
+ struct cwdinfo *cwdi = l-l_proc-p_cwdi;
switch (ms) {
case 'p':
@@ -137,6 +137,15 @@ 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 (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;
@@ -188,7 +197,7 @@ ptyfs__getvattr(struct mount *mp, struct
void
ptyfs_init(void)
{
-
+ LIST_INIT(ptm_ptyfspty.arg);
malloc_type_attach(M_PTYFSMNT);
malloc_type_attach(M_PTYFSTMP);
ptyfs_hashinit();
@@ -218,7 +227,7 @@ ptyfs_mount(struct mount *mp, const char
{
struct lwp *l = curlwp;
int
Re: Enhance ptyfs to handle multiple instances.
Hello!
This patch introduces subject.
Some code refactoring, because we have more stronger ptm - ptyfs binding.
BSD pty compatibility improvement.
Main explanation you can see in comments inside.
Also I am not sure about, how, correctly release unused vnode and return it
for system and call reclaim. Before anybody call for it getnewvnode.
fs/ptyfs/ptyfs.h| 21 +
fs/ptyfs/ptyfs_subr.c |2 --
fs/ptyfs/ptyfs_vfsops.c | 37 +
fs/ptyfs/ptyfs_vnops.c | 26 --
kern/tty_bsdpty.c |4 ++--
kern/tty_ptm.c | 31 +++
sys/pty.h | 21 -
7 files changed, 99 insertions(+), 43 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs.h
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs.h,v
retrieving revision 1.6
diff -u -p -r1.6 ptyfs.h
--- fs/ptyfs/ptyfs.h 24 Mar 2014 20:48:08 - 1.6
+++ fs/ptyfs/ptyfs.h 1 Apr 2014 19:00:15 -
@@ -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;
@@ -113,6 +115,25 @@ struct ptyfsmount {
#define VFSTOPTY(mp) ((struct ptyfsmount *)(mp)-mnt_data)
+LIST_HEAD(ptyfs_lhmp, ptyfsmount);
+
+/*
+ * 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.
+ */
+struct ptm_pty {
+ int (*allocvp)(struct mount *, struct lwp *, struct vnode **, dev_t,
+ char);
+ int (*makename)(struct mount *, struct lwp *, char *, size_t, dev_t, char);
+ void (*getvattr)(struct mount *, struct lwp *, struct vattr *);
+ struct ptyfs_lhmp *arg;
+};
+
+dev_t pty_makedev(char, int);
+int pty_vn_open(struct vnode *, struct lwp *);
+struct ptm_pty *pty_sethandler(struct ptm_pty *);
+
#endif /* _KERNEL */
struct ptyfs_args {
Index: fs/ptyfs/ptyfs_subr.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.6
diff -u -p -r1.6 ptyfs_subr.c
--- fs/ptyfs/ptyfs_subr.c 28 Mar 2014 10:53:57 - 1.6
+++ fs/ptyfs/ptyfs_subr.c 1 Apr 2014 19:00:15 -
@@ -86,8 +86,6 @@ __KERNEL_RCSID(0, $NetBSD: ptyfs_subr.c
#include sys/namei.h
#include sys/filedesc.h
#include sys/select.h
-#include sys/tty.h
-#include sys/pty.h
#include sys/kauth.h
#include sys/lwp.h
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.13
diff -u -p -r1.13 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 28 Mar 2014 10:53:57 - 1.13
+++ fs/ptyfs/ptyfs_vfsops.c 1 Apr 2014 19:00:15 -
@@ -54,8 +54,6 @@ __KERNEL_RCSID(0, $NetBSD: ptyfs_vfsops
#include sys/syslog.h
#include sys/select.h
#include sys/filedesc.h
-#include sys/tty.h
-#include sys/pty.h
#include sys/kauth.h
#include sys/module.h
@@ -83,12 +81,13 @@ static void ptyfs__getvattr(struct mount
*/
struct ptm_pty *ptyfs_save_ptm;
static int ptyfs_count;
+static struct ptyfs_lhmp mph;
struct ptm_pty ptm_ptyfspty = {
ptyfs__allocvp,
ptyfs__makename,
ptyfs__getvattr,
- NULL
+ mph
};
static const char *
@@ -130,6 +129,7 @@ ptyfs__makename(struct mount *mp, struct
{
size_t len;
const char *np;
+ struct cwdinfo *cwdi = l-l_proc-p_cwdi;
switch (ms) {
case 'p':
@@ -137,6 +137,15 @@ 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 (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;
@@ -188,7 +197,7 @@ ptyfs__getvattr(struct mount *mp, struct
void
ptyfs_init(void)
{
-
+ LIST_INIT(ptm_ptyfspty.arg);
malloc_type_attach(M_PTYFSMNT);
malloc_type_attach(M_PTYFSTMP);
ptyfs_hashinit();
@@ -218,7 +227,7 @@ ptyfs_mount(struct mount *mp, const char
{
struct lwp *l = curlwp;
int error = 0;
- struct ptyfsmount *pmnt;
+ struct ptyfsmount *pmnt, *pmnt_next;
struct ptyfs_args *args = data;
if (*data_len != sizeof *args *data_len != OSIZE)
@@ -274,10 +283,16 @@ ptyfs_mount(struct mount *mp, const char
return error;
}
- /* Point pty access to us */
+ pmnt-pmnt_mp = mp;
if (ptyfs_count == 0) {
- ptm_ptyfspty.arg = mp;
+ LIST_INSERT_HEAD(ptm_ptyfspty.arg, pmnt, pmnt_le);
+ /* Point pty access to us */
ptyfs_save_ptm = pty_sethandler(ptm_ptyfspty);
+ } else {
+ pmnt_next
Re: Enhance ptyfs to handle multiple instances.
Hello!
Maybe you skipped:
Minor corrections readdir and lookup for multi-mountpoint use.
ptyfs_vnops.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
Resending.
Also main patch for subject.
I didn't want locate many code in ptm driver, but in real world,
it was the most suitable place(performance, flexible ..).
Most of changes were tied with code refactoring. I had added only one
ptyfs_getmp().
In the near future we will need only decide how will keep,get list of mount
points?
fs/ptyfs/ptyfs_subr.c |2 -
fs/ptyfs/ptyfs_vfsops.c | 15 --
kern/tty_bsdpty.c | 18 ++--
kern/tty_ptm.c | 69 +++-
kern/tty_pty.c | 12 ++--
sys/pty.h | 23 ++--
6 files changed, 92 insertions(+), 47 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs_vnops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.3
diff -u -p -r1.3 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c 24 Mar 2014 20:48:09 - 1.3
+++ fs/ptyfs/ptyfs_vnops.c 26 Mar 2014 08:49:47 -
@@ -616,7 +616,8 @@ ptyfs_lookup(void *v)
pty = atoi(pname, cnp-cn_namelen);
- if (pty 0 || pty = npty || pty_isfree(pty, 1))
+ if (pty 0 || pty = npty || pty_isfree(pty, 1) ||
+ ptyfs_used_get(PTYFSptc, pty, dvp-v_mount, 0) == NULL)
break;
error = ptyfs_allocvp(dvp-v_mount, vpp, PTYFSpts, pty,
@@ -711,7 +712,8 @@ ptyfs_readdir(void *v)
}
for (; uio-uio_resid = UIO_MX i npty; i++) {
/* check for used ptys */
- if (ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
+ if (pty_isfree(i - 2, 1) ||
+ ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
continue;
dp-d_fileno = PTYFS_FILENO(i - 2, PTYFSpts);
Index: fs/ptyfs/ptyfs_subr.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -p -r1.4 -r1.5
--- fs/ptyfs/ptyfs_subr.c 26 Mar 2014 10:29:29 - 1.4
+++ fs/ptyfs/ptyfs_subr.c 26 Mar 2014 21:32:13 - 1.5
@@ -139,7 +139,7 @@ ptyfs_getinfo(struct ptyfsnode *ptyfs, s
* from the inode
*/
if ((error = (*ptyfs_save_ptm-makename)(
- ptyfs_save_ptm, l, ttyname, sizeof(ttyname),
+ NULL, l, ttyname, sizeof(ttyname),
ptyfs-ptyfs_pty, ptyfs-ptyfs_type == PTYFSpts ? 't'
: 'p')) != 0)
goto out;
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.10
diff -u -p -r1.10 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 24 Mar 2014 20:48:08 - 1.10
+++ fs/ptyfs/ptyfs_vfsops.c 27 Mar 2014 07:53:55 -
@@ -72,11 +72,11 @@ VFS_PROTOS(ptyfs);
static struct sysctllog *ptyfs_sysctl_log;
-static int ptyfs__allocvp(struct ptm_pty *, struct lwp *, struct vnode **,
+static int ptyfs__allocvp(struct mount *, struct lwp *, struct vnode **,
dev_t, char);
-static int ptyfs__makename(struct ptm_pty *, struct lwp *, char *, size_t,
+static int ptyfs__makename(struct mount *, struct lwp *, char *, size_t,
dev_t, char);
-static void ptyfs__getvattr(struct ptm_pty *, struct lwp *, struct vattr *);
+static void ptyfs__getvattr(struct mount *, struct lwp *, struct vattr *);
/*
* ptm glue: When we mount, we make ptm point to us.
@@ -125,10 +125,9 @@ out:
}
static int
-ptyfs__makename(struct ptm_pty *pt, struct lwp *l, char *tbuf, size_t bufsiz,
+ptyfs__makename(struct mount *mp, struct lwp *l, char *tbuf, size_t bufsiz,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
size_t len;
const char *np;
@@ -154,10 +153,9 @@ ptyfs__makename(struct ptm_pty *pt, stru
static int
/*ARGSUSED*/
-ptyfs__allocvp(struct ptm_pty *pt, struct lwp *l, struct vnode **vpp,
+ptyfs__allocvp(struct mount *mp, struct lwp *l, struct vnode **vpp,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
ptyfstype type;
switch (ms) {
@@ -176,9 +174,8 @@ ptyfs__allocvp(struct ptm_pty *pt, struc
static void
-ptyfs__getvattr(struct ptm_pty *pt, struct lwp *l, struct vattr *vattr)
+ptyfs__getvattr(struct mount *mp, struct lwp *l, struct vattr *vattr)
{
- struct mount *mp = pt-arg;
struct ptyfsmount *pmnt = VFSTOPTY(mp);
vattr_null(vattr);
/* get real uid */
Index: kern/tty_bsdpty.c
===
RCS file: /cvsil/nbcur/src/sys/kern/tty_bsdpty.c,v
retrieving revision 1.2
diff -u -p -r1.2 tty_bsdpty.c
--- kern/tty_bsdpty.c 26 Mar 2014 10:29:29 - 1.2
+++ kern/tty_bsdpty.c 27 Mar 2014 07:53:55 -
@@ -31,6 +31,7 @@ __KERNEL_RCSID(0, $NetBSD: tty_bsdpty.c
#include opt_ptm.h
+#ifndef NO_DEV_PTM
#ifdef COMPAT_BSDPTY
/* bsd tty implementation for pty multiplexor driver /dev/ptm{,x} */
@@ -68,11 +69,11 @@ __KERNEL_RCSID(0, $NetBSD: tty_bsdpty.c
#define TTY_OLD_SUFFIX
Re: Enhance ptyfs to handle multiple instances.
On 27.03.2014 12:51, Ilya Zykov wrote:
Hello!
Maybe you skipped:
Minor corrections readdir and lookup for multi-mountpoint use.
ptyfs_vnops.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
Resending.
Also main patch for subject.
I didn't want locate many code in ptm driver, but in real world,
it was the most suitable place(performance, flexible ..).
Most of changes were tied with code refactoring. I had added only one
ptyfs_getmp().
In the near future we will need only decide how will keep,get list of mount
points?
fs/ptyfs/ptyfs_subr.c |2 -
fs/ptyfs/ptyfs_vfsops.c | 15 --
kern/tty_bsdpty.c | 18 ++--
kern/tty_ptm.c | 69
+++-
kern/tty_pty.c | 12 ++--
sys/pty.h | 23 ++--
6 files changed, 92 insertions(+), 47 deletions(-)
Ilya.
Little BUG fix.
fs/ptyfs/ptyfs_subr.c |2 -
fs/ptyfs/ptyfs_vfsops.c | 15 --
kern/tty_bsdpty.c | 18 ++--
kern/tty_ptm.c | 72
kern/tty_pty.c | 12 ++--
sys/pty.h | 23 ++-
6 files changed, 95 insertions(+), 47 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs_subr.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -p -r1.4 -r1.5
--- fs/ptyfs/ptyfs_subr.c 26 Mar 2014 10:29:29 - 1.4
+++ fs/ptyfs/ptyfs_subr.c 26 Mar 2014 21:32:13 - 1.5
@@ -139,7 +139,7 @@ ptyfs_getinfo(struct ptyfsnode *ptyfs, s
* from the inode
*/
if ((error = (*ptyfs_save_ptm-makename)(
- ptyfs_save_ptm, l, ttyname, sizeof(ttyname),
+ NULL, l, ttyname, sizeof(ttyname),
ptyfs-ptyfs_pty, ptyfs-ptyfs_type == PTYFSpts ? 't'
: 'p')) != 0)
goto out;
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.10
diff -u -p -r1.10 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 24 Mar 2014 20:48:08 - 1.10
+++ fs/ptyfs/ptyfs_vfsops.c 27 Mar 2014 13:48:13 -
@@ -72,11 +72,11 @@ VFS_PROTOS(ptyfs);
static struct sysctllog *ptyfs_sysctl_log;
-static int ptyfs__allocvp(struct ptm_pty *, struct lwp *, struct vnode **,
+static int ptyfs__allocvp(struct mount *, struct lwp *, struct vnode **,
dev_t, char);
-static int ptyfs__makename(struct ptm_pty *, struct lwp *, char *, size_t,
+static int ptyfs__makename(struct mount *, struct lwp *, char *, size_t,
dev_t, char);
-static void ptyfs__getvattr(struct ptm_pty *, struct lwp *, struct vattr *);
+static void ptyfs__getvattr(struct mount *, struct lwp *, struct vattr *);
/*
* ptm glue: When we mount, we make ptm point to us.
@@ -125,10 +125,9 @@ out:
}
static int
-ptyfs__makename(struct ptm_pty *pt, struct lwp *l, char *tbuf, size_t bufsiz,
+ptyfs__makename(struct mount *mp, struct lwp *l, char *tbuf, size_t bufsiz,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
size_t len;
const char *np;
@@ -154,10 +153,9 @@ ptyfs__makename(struct ptm_pty *pt, stru
static int
/*ARGSUSED*/
-ptyfs__allocvp(struct ptm_pty *pt, struct lwp *l, struct vnode **vpp,
+ptyfs__allocvp(struct mount *mp, struct lwp *l, struct vnode **vpp,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
ptyfstype type;
switch (ms) {
@@ -176,9 +174,8 @@ ptyfs__allocvp(struct ptm_pty *pt, struc
static void
-ptyfs__getvattr(struct ptm_pty *pt, struct lwp *l, struct vattr *vattr)
+ptyfs__getvattr(struct mount *mp, struct lwp *l, struct vattr *vattr)
{
- struct mount *mp = pt-arg;
struct ptyfsmount *pmnt = VFSTOPTY(mp);
vattr_null(vattr);
/* get real uid */
Index: kern/tty_bsdpty.c
===
RCS file: /cvsil/nbcur/src/sys/kern/tty_bsdpty.c,v
retrieving revision 1.2
diff -u -p -r1.2 tty_bsdpty.c
--- kern/tty_bsdpty.c 26 Mar 2014 10:29:29 - 1.2
+++ kern/tty_bsdpty.c 27 Mar 2014 13:48:14 -
@@ -31,6 +31,7 @@ __KERNEL_RCSID(0, $NetBSD: tty_bsdpty.c
#include opt_ptm.h
+#ifndef NO_DEV_PTM
#ifdef COMPAT_BSDPTY
/* bsd tty implementation for pty multiplexor driver /dev/ptm{,x} */
@@ -68,11 +69,11 @@ __KERNEL_RCSID(0, $NetBSD: tty_bsdpty.c
#define TTY_OLD_SUFFIX 0123456789abcdef
#define TTY_NEW_SUFFIX ghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ
-static int pty_makename(struct ptm_pty *, struct lwp *, char *, size_t, dev_t,
+static int pty_makename(struct mount *, struct lwp *, char *, size_t, dev_t,
char);
-static int pty_allocvp(struct ptm_pty *, struct lwp *, struct vnode **,
+static int pty_allocvp(struct mount *, struct lwp *, struct vnode **,
dev_t, char);
-static void pty_getvattr(struct ptm_pty *, struct lwp *, struct vattr *);
+static void pty_getvattr(struct mount *, struct lwp
Re: Enhance ptyfs to handle multiple instances.
On Mar 27, 5:53pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | This is a multi-part message in MIME format. | --040300020609040305030709 | Content-Type: text/plain; charset=ISO-8859-1 | Content-Transfer-Encoding: 7bit | | On 27.03.2014 12:51, Ilya Zykov wrote: | Hello! | Maybe you skipped: | Minor corrections readdir and lookup for multi-mountpoint use. | | ptyfs_vnops.c |6 -- | 1 file changed, 4 insertions(+), 2 deletions(-) | | Resending. | | Also main patch for subject. | I didn't want locate many code in ptm driver, but in real world, | it was the most suitable place(performance, flexible ..). | Most of changes were tied with code refactoring. I had added only one ptyfs_getmp(). | In the near future we will need only decide how will keep,get list of mount points? | Yes, we need to think about it. In the general case there won't be many so it should not be that hard. christos
Re: Enhance ptyfs to handle multiple instances.
| On 27.03.2014 12:51, Ilya Zykov wrote:
| Hello!
| Maybe you skipped:
| Minor corrections readdir and lookup for multi-mountpoint use.
|
| ptyfs_vnops.c |6 --
| 1 file changed, 4 insertions(+), 2 deletions(-)
Please, don't forget this, otherwise readdir returns released(free), but still
hashed inode numbers.
Ilya.
Index: fs/ptyfs/ptyfs_vnops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.3
diff -u -p -r1.3 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c 24 Mar 2014 20:48:09 - 1.3
+++ fs/ptyfs/ptyfs_vnops.c 26 Mar 2014 08:49:47 -
@@ -616,7 +616,8 @@ ptyfs_lookup(void *v)
pty = atoi(pname, cnp-cn_namelen);
- if (pty 0 || pty = npty || pty_isfree(pty, 1))
+ if (pty 0 || pty = npty || pty_isfree(pty, 1) ||
+ ptyfs_used_get(PTYFSptc, pty, dvp-v_mount, 0) == NULL)
break;
error = ptyfs_allocvp(dvp-v_mount, vpp, PTYFSpts, pty,
@@ -711,7 +712,8 @@ ptyfs_readdir(void *v)
}
for (; uio-uio_resid = UIO_MX i npty; i++) {
/* check for used ptys */
- if (ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
+ if (pty_isfree(i - 2, 1) ||
+ ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
continue;
dp-d_fileno = PTYFS_FILENO(i - 2, PTYFSpts);
Re: Enhance ptyfs to handle multiple instances.
On Mar 28, 12:37am, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | Please, don't forget this, otherwise readdir returns released(free), but still hashed inode numbers. | Got it, thanks! christos
Re: Enhance ptyfs to handle multiple instances.
Hello!
Minor corrections readdir and lookup for multi-mountpoint use.
ptyfs_vnops.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs_vnops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.3
diff -u -p -r1.3 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c 24 Mar 2014 20:48:09 - 1.3
+++ fs/ptyfs/ptyfs_vnops.c 26 Mar 2014 08:49:47 -
@@ -616,7 +616,8 @@ ptyfs_lookup(void *v)
pty = atoi(pname, cnp-cn_namelen);
- if (pty 0 || pty = npty || pty_isfree(pty, 1))
+ if (pty 0 || pty = npty || pty_isfree(pty, 1) ||
+ ptyfs_used_get(PTYFSptc, pty, dvp-v_mount, 0) == NULL)
break;
error = ptyfs_allocvp(dvp-v_mount, vpp, PTYFSpts, pty,
@@ -711,7 +712,8 @@ ptyfs_readdir(void *v)
}
for (; uio-uio_resid = UIO_MX i npty; i++) {
/* check for used ptys */
- if (ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
+ if (pty_isfree(i - 2, 1) ||
+ ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
continue;
dp-d_fileno = PTYFS_FILENO(i - 2, PTYFSpts);
Re: Enhance ptyfs to handle multiple instances.
On Mar 26, 2:09pm, net...@izyk.ru (Ilya Zykov) wrote:
-- Subject: Re: Enhance ptyfs to handle multiple instances.
| Index: fs/ptyfs/ptyfs_subr.c
| ===
| RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
| retrieving revision 1.3
| diff -u -r1.3 ptyfs_subr.c
| --- fs/ptyfs/ptyfs_subr.c 24 Mar 2014 20:48:08 - 1.3
| +++ fs/ptyfs/ptyfs_subr.c 26 Mar 2014 09:44:44 -
| @@ -116,7 +116,7 @@
| static void
| ptyfs_getinfo(struct ptyfsnode *ptyfs, struct lwp *l)
| {
| - extern struct ptm_pty *ptyfs_save_ptm, ptm_ptyfspty;
| + extern struct ptm_pty *ptyfs_save_ptm;
|
| if (ptyfs-ptyfs_type == PTYFSroot) {
| ptyfs-ptyfs_mode = S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP|
| @@ -126,7 +126,7 @@
| ptyfs-ptyfs_mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|
| S_IROTH|S_IWOTH;
|
| - if (ptyfs_save_ptm != NULL ptyfs_save_ptm != ptm_ptyfspty) {
| + if (ptyfs_save_ptm != NULL) {
| int error;
| struct pathbuf *pb;
| struct nameidata nd;
Ok,
| Index: kern/tty_bsdpty.c
| ===
| RCS file: /cvsil/nbcur/src/sys/kern/tty_bsdpty.c,v
| retrieving revision 1.1.1.1
| diff -u -r1.1.1.1 tty_bsdpty.c
| --- kern/tty_bsdpty.c 4 Mar 2014 18:16:04 - 1.1.1.1
| +++ kern/tty_bsdpty.c 26 Mar 2014 09:44:44 -
| @@ -121,7 +121,7 @@
| struct nameidata nd;
| char name[TTY_NAMESIZE];
|
| - error = (*ptm-makename)(ptm, l, name, sizeof(name), dev, ms);
| + error = pty_makename(ptm, l, name, sizeof(name), dev, ms);
| if (error)
| return error;
|
Are you sure about this one? It is used when ptyfs is mounted and you have
old pty nodes around (so you get consistent names).
christos
Re: Enhance ptyfs to handle multiple instances.
PTYFS has dependency from ptm driver. If config has NO_DEV_PTM, PTYFS isn't compiled. PTYFS is useless without ptm. How, better, this condition is fixed in config files?
Re: Enhance ptyfs to handle multiple instances.
On Mar 26, 6:36pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | PTYFS has dependency from ptm driver. | If config has NO_DEV_PTM, PTYFS isn't compiled. | PTYFS is useless without ptm. | | How, better, this condition is fixed in config files? Add ' !no_dev_ptm' next to the ptyfs files in sys/fs/ptyfs/files.ptyfs? I am not sure... Is there a way to bail out configuration of a filesystem if a feature is missing, for example no coda if no venus? christos
Re: Enhance ptyfs to handle multiple instances.
In article 5332dc47.1060...@izyk.ru, Ilya Zykov net...@izyk.ru wrote: | | - error = (*ptm-makename)(ptm, l, name, sizeof(name), dev, ms); | + error = pty_makename(ptm, l, name, sizeof(name), dev, ms); |if (error) |return error; | Are you sure about this one? It is used when ptyfs is mounted and you have old pty nodes around (so you get consistent names). christos I think pty_allocvp can be invoked only when ptm == ptm_bsdpty, therefore: ptm-allocvp == pty_allocvp, ptm-makename == pty_makename. ptm-getvattr == pty_getvattr. In what condition and who can invokes pty_allocvp, when ptm == ptm_ptyfspty? Yes, that is true, ignore me. christos
Re: Enhance ptyfs to handle multiple instances.
On Mon, Mar 24, 2014 at 10:49:15AM -0400, Christos Zoulas wrote: On Mar 24, 5:46pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | Hello! | | Please, tell me know if I wrong. | In general case I can't find(easy), from driver, where its device file located on file system, | its vnode or its directory vnode where this file located. | Such files can be many and I can't find what file used for current operation. | Maybe anybody had being attempted get this info from the driver? You can't find from the driver where the device node file is located in the filesystem, as well as you cannot reliably find from the vnode of the device node the filesystem path. There could be many device nodes that satisfy the criteria (you can make your own tty node with mknod) FWIW SYSV ptys (etc) would be created as a 'clone', a /dev entry created/found with the required path and the correct major/minor, and then reopened through the filesystem entry. stat() on the /dev entry and fstat() on the fd would then agree (probably disk partition and inode?). This didn't help you find the entry - but it would tell you when you'd found the correct one. OTOH ttyname(3) is probably best implemented with a pair of ioctls. Although chroot() probably complicates things. David -- David Laight: da...@l8s.co.uk
Re: Enhance ptyfs to handle multiple instances.
Hello! Please, tell me know if I wrong. In general case I can't find(easy), from driver, where its device file located on file system, its vnode or its directory vnode where this file located. Such files can be many and I can't find what file used for current operation. Maybe anybody had being attempted get this info from the driver? Ilya.
Re: Enhance ptyfs to handle multiple instances.
On Mar 24, 5:46pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | Hello! | | Please, tell me know if I wrong. | In general case I can't find(easy), from driver, where its device file located on file system, | its vnode or its directory vnode where this file located. | Such files can be many and I can't find what file used for current operation. | Maybe anybody had being attempted get this info from the driver? You can't find from the driver where the device node file is located in the filesystem, as well as you cannot reliably find from the vnode of the device node the filesystem path. There could be many device nodes that satisfy the criteria (you can make your own tty node with mknod) christos
Re: Enhance ptyfs to handle multiple instances.
You can't find from the driver where the device node file is located OK, I thought so. Thank you.
Re: Enhance ptyfs to handle multiple instances.
I don't understand why you want to get rid of the mountpoint arg inside the pty structure. It certainly makes things faster, and the pty can't be shared... christos Sorry, but I don't understand too, what structure do you mean exactly and how. Ilya.
Re: Enhance ptyfs to handle multiple instances.
On Mar 22, 3:50pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | | I don't understand why you want to get rid of the mountpoint arg inside | the pty structure. It certainly makes things faster, and the pty can't | be shared... | | christos | | | Sorry, but I don't understand too, what structure do you mean exactly and how. The mountpoint inside ptm_pty. Perhaps by having separate instances in the ptm driver? christos
Re: Enhance ptyfs to handle multiple instances.
In article 532def5e.2040...@izyk.ru, Ilya Zykov net...@izyk.ru wrote: The mountpoint inside ptm_pty. Perhaps by having separate instances in the ptm driver? christos I think, it's not better. I can do so, but: 1. Now we have only 2 instances ptm_pty, one for ptyfs one for bsdpty and use its mainly for switch from one to other(we will have ptm_pty array). 2. Now we keep local ptyfs' data pointer(mp) inside external ptm_pty it's mistaken way(IMHO). We have useless ping pong local data. Maybe it is conceived for other goal. Easier keep it in local static pointer and don't pass it in parameters every function call. 3. I don't want dispose ptyfs code inside ptm driver. 4. We will have export ptyfs__getpath(). or 5. Use ptm minor numbers for differentiating factor(or other differentiating factor), maybe, useless work, because user space programs use only one instance. Really multiple instances are needed only for chroot. Also it complicates user space programs, but give more productivity(we don't need look up mount point). Ok this is fine for now then. christos
Re: Enhance ptyfs to handle multiple instances.
Hello!
Correct ptyfs_readdir for multi mount points use.
ptyfs.h |1 +
ptyfs_subr.c |3 +--
ptyfs_vnops.c |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs.h
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs.h,v
retrieving revision 1.4
diff -u -r1.4 ptyfs.h
--- fs/ptyfs/ptyfs.h 19 Mar 2014 04:53:36 - 1.4
+++ fs/ptyfs/ptyfs.h 21 Mar 2014 09:29:12 -
@@ -148,6 +148,7 @@
#define PTYFSTOV(ptyfs) ((ptyfs)-ptyfs_vnode)
int ptyfs_freevp(struct vnode *);
+struct vnode *ptyfs_used_get(ptyfstype, int, struct mount *, int);
int ptyfs_allocvp(struct mount *, struct vnode **, ptyfstype, int,
struct lwp *);
void ptyfs_hashinit(void);
Index: fs/ptyfs/ptyfs_subr.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_subr.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ptyfs_subr.c
--- fs/ptyfs/ptyfs_subr.c 4 Mar 2014 18:16:03 - 1.1.1.1
+++ fs/ptyfs/ptyfs_subr.c 21 Mar 2014 09:29:12 -
@@ -105,7 +105,6 @@
static void ptyfs_hashins(struct ptyfsnode *);
static void ptyfs_hashrem(struct ptyfsnode *);
-static struct vnode *ptyfs_used_get(ptyfstype, int, struct mount *, int);
static struct ptyfsnode *ptyfs_free_get(ptyfstype, int, struct lwp *);
static void ptyfs_rehash(kmutex_t *, struct ptyfs_hashhead **,
@@ -186,7 +185,7 @@
* allocate a ptyfsnode/vnode pair. the vnode is
* referenced, and locked.
*
- * the pid, ptyfs_type, and mount point uniquely
+ * the pty, ptyfs_type, and mount point uniquely
* identify a ptyfsnode. the mount point is needed
* because someone might mount this filesystem
* twice.
Index: fs/ptyfs/ptyfs_vnops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vnops.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ptyfs_vnops.c
--- fs/ptyfs/ptyfs_vnops.c 4 Mar 2014 18:16:03 - 1.1.1.1
+++ fs/ptyfs/ptyfs_vnops.c 21 Mar 2014 09:29:12 -
@@ -711,7 +711,7 @@
}
for (; uio-uio_resid = UIO_MX i npty; i++) {
/* check for used ptys */
- if (pty_isfree(i - 2, 1))
+ if (ptyfs_used_get(PTYFSptc, i - 2, vp-v_mount, 0) == NULL)
continue;
dp-d_fileno = PTYFS_FILENO(i - 2, PTYFSpts);
Re: Enhance ptyfs to handle multiple instances.
In article 532c0718.3020...@izyk.ru, Ilya Zykov net...@izyk.ru wrote: -=-=-=-=-=- Hello! Correct ptyfs_readdir for multi mount points use. committed. christos
Re: Enhance ptyfs to handle multiple instances.
DONE.
:)
If seriously, it's first working prototype for comments and objections.
It's working as follow:
Mount first ptyfs instance in /dev/pts(or other path) you can get access to
master side
through ptm{x} device.
Mount second ptyfs instance inside chroot(Example: /var/chroot/test/dev/pts),
create ptm{x}
device inside chroot(Ex. /var/chroot/test/dev/ptm{x}).
Chroot: chroot /var/chroot/test /rescue/sh,
now you can see only second instance in /dev/pts.
ptyfs_vfsops.c | 47 +++
1 file changed, 35 insertions(+), 12 deletions(-)
I'm leaving for the weekend till Monday.
Best regards.
Ilya.
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.9
diff -u -r1.9 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 19 Mar 2014 21:00:16 - 1.9
+++ fs/ptyfs/ptyfs_vfsops.c 21 Mar 2014 17:55:54 -
@@ -83,6 +83,8 @@
*/
struct ptm_pty *ptyfs_save_ptm;
static int ptyfs_count;
+#define PTYFS_MINSTANCE 16
+static struct mount *ptyfs_mp[PTYFS_MINSTANCE] = { NULL };
struct ptm_pty ptm_ptyfspty = {
ptyfs__allocvp,
@@ -124,13 +126,25 @@
return rv;
}
+static struct mount *
+ptyfs__mpget(struct lwp *l) {
+ int i;
+ for (i = 0; i PTYFS_MINSTANCE; i++)
+ if (ptyfs_mp[i] != NULL
+ ptyfs__getpath(l, ptyfs_mp[i]) != NULL)
+ return ptyfs_mp[i];
+ return NULL;
+}
+
static int
ptyfs__makename(struct ptm_pty *pt, struct lwp *l, char *tbuf, size_t bufsiz,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
size_t len;
const char *np;
+ struct mount *mp = ptyfs__mpget(l);
+ if (mp == NULL)
+ return EOPNOTSUPP;
switch (ms) {
case 'p':
@@ -157,8 +171,10 @@
ptyfs__allocvp(struct ptm_pty *pt, struct lwp *l, struct vnode **vpp,
dev_t dev, char ms)
{
- struct mount *mp = pt-arg;
ptyfstype type;
+ struct mount *mp = ptyfs__mpget(l);
+ if (mp == NULL)
+ return EOPNOTSUPP;
switch (ms) {
case 'p':
@@ -178,7 +194,7 @@
static void
ptyfs__getvattr(struct ptm_pty *pt, struct lwp *l, struct vattr *vattr)
{
- struct mount *mp = pt-arg;
+ struct mount *mp = ptyfs__mpget(l);
struct ptyfsmount *pmnt = VFSTOPTY(mp);
vattr_null(vattr);
/* get real uid */
@@ -220,7 +236,7 @@
ptyfs_mount(struct mount *mp, const char *path, void *data, size_t *data_len)
{
struct lwp *l = curlwp;
- int error = 0;
+ int i, error = 0;
struct ptyfsmount *pmnt;
struct ptyfs_args *args = data;
@@ -247,11 +263,9 @@
return 0;
}
-#if 0
- /* Don't allow more than one mount */
- if (ptyfs_count)
+ /* Don't allow more than PTYFS_MINSTANCE mount */
+ if (PTYFS_MINSTANCE = ptyfs_count)
return EBUSY;
-#endif
if (mp-mnt_flag MNT_UPDATE)
return EOPNOTSUPP;
@@ -278,10 +292,13 @@
}
/* Point pty access to us */
- if (ptyfs_count == 0) {
- ptm_ptyfspty.arg = mp;
+ if (ptyfs_count == 0)
ptyfs_save_ptm = pty_sethandler(ptm_ptyfspty);
- }
+ for (i = 0; i PTYFS_MINSTANCE; i++)
+ if (ptyfs_mp[i] == NULL) {
+ ptyfs_mp[i] = mp;
+ break;
+ }
ptyfs_count++;
return 0;
}
@@ -297,7 +314,7 @@
int
ptyfs_unmount(struct mount *mp, int mntflags)
{
- int error;
+ int error, i;
int flags = 0;
if (mntflags MNT_FORCE)
@@ -306,6 +323,12 @@
if ((error = vflush(mp, 0, flags)) != 0)
return error;
+ for (i = 0; i PTYFS_MINSTANCE; i++)
+ if (ptyfs_mp[i] == mp) {
+ ptyfs_mp[i] = NULL;
+ break;
+ }
+
ptyfs_count--;
if (ptyfs_count == 0) {
/* Restore where pty access was pointing */
Re: Enhance ptyfs to handle multiple instances.
On Mar 21, 10:23pm, net...@izyk.ru (Ilya Zykov) wrote:
-- Subject: Re: Enhance ptyfs to handle multiple instances.
| If seriously, it's first working prototype for comments and objections.
|
| It's working as follow:
|
| Mount first ptyfs instance in /dev/pts(or other path) you can get access to
master side
| through ptm{x} device.
|
| Mount second ptyfs instance inside chroot(Example: /var/chroot/test/dev/pts),
create ptm{x}
| device inside chroot(Ex. /var/chroot/test/dev/ptm{x}).
| Chroot: chroot /var/chroot/test /rescue/sh,
| now you can see only second instance in /dev/pts.
|
| I'm leaving for the weekend till Monday.
Enjoy!
I don't understand why you want to get rid of the mountpoint arg inside
the pty structure. It certainly makes things faster, and the pty can't
be shared...
christos
Re: Enhance ptyfs to handle multiple instances.
Hello!
For not accumulate many changes and keep patch clear.
I am sending some error fix and modifications for future work and discussion
too.
Please, if possible, include it in current tree.
Little explanations:
1. We shouldn't mount more than one ptyfs.(dependency from unmount order).
Panic, if unmount first mount without the rest.
2. Some changes for correct working inside chroot:
If mount point is outside chroot we can't any more get pts pathname.
Correct getting pathname if mount point inside chroot.
fs/ptyfs/ptyfs_vfsops.c | 21 ++---
kern/tty_ptm.c |9 -
2 files changed, 22 insertions(+), 8 deletions(-)
Ilya.
Index: nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c
diff -u nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.1.1.1 nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.7
--- nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c:1.1.1.1 Tue Mar 4 22:16:03 2014
+++ nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c Wed Mar 19 15:41:47 2014
@@ -109,14 +109,16 @@
buf = malloc(MAXBUF, M_TEMP, M_WAITOK);
bp = buf + MAXBUF;
*--bp = '\0';
- error = getcwd_common(cwdi-cwdi_rdir, rootvnode, bp,
+ error = getcwd_common(mp-mnt_vnodecovered, cwdi-cwdi_rdir, bp,
buf, MAXBUF / 2, 0, l);
- if (error) /* XXX */
+ if (error) { /* Mount point is out of rdir */
+ rv = NULL;
goto out;
+ }
len = strlen(bp);
if (len sizeof(mp-mnt_stat.f_mntonname)) /* XXX */
- rv += len;
+ rv += strlen(rv) - len;
out:
free(buf, M_TEMP);
return rv;
@@ -128,6 +130,7 @@
{
struct mount *mp = pt-arg;
size_t len;
+ const char *np;
switch (ms) {
case 'p':
@@ -135,9 +138,13 @@
len = snprintf(tbuf, bufsiz, /dev/null);
break;
case 't':
- len = snprintf(tbuf, bufsiz, %s/%llu, ptyfs__getpath(l, mp),
- (unsigned long long)minor(dev));
- break;
+ np = ptyfs__getpath(l, mp);
+ if ( np ) {
+ len = snprintf(tbuf, bufsiz, %s/%llu, np,
+(unsigned long long)minor(dev));
+ break;
+ }
+ return EOPNOTSUPP;
default:
return EINVAL;
}
@@ -241,7 +248,7 @@
return 0;
}
-#if 0
+#if 1
/* Don't allow more than one mount */
if (ptyfs_count)
return EBUSY;
Index: nbcur/src/sys/kern/tty_ptm.c
diff -u nbcur/src/sys/kern/tty_ptm.c:1.1.1.2 nbcur/src/sys/kern/tty_ptm.c:1.2
--- nbcur/src/sys/kern/tty_ptm.c:1.1.1.2 Mon Mar 17 15:46:10 2014
+++ nbcur/src/sys/kern/tty_ptm.c Wed Mar 19 12:59:47 2014
@@ -381,7 +381,9 @@
goto bad;
/* now, put the indices and names into struct ptmget */
- return pty_fill_ptmget(l, newdev, cfd, sfd, data);
+ if ((error = pty_fill_ptmget(l, newdev, cfd, sfd, data)) != 0 )
+ break; /* goto bad2 */
+ return 0;
default:
#ifdef COMPAT_60
error = compat_60_ptmioctl(dev, cmd, data, flag, l);
@@ -391,6 +393,11 @@
DPRINTF((ptmioctl EINVAL\n));
return EINVAL;
}
+/* bad2: close sfd too */
+ fp = fd_getfile(sfd);
+ if (fp != NULL) {
+ fd_close(sfd);
+ }
bad:
fp = fd_getfile(cfd);
if (fp != NULL) {
Re: Enhance ptyfs to handle multiple instances.
| *--bp = '\0'; | - error = getcwd_common(cwdi-cwdi_rdir, rootvnode, bp, | + error = getcwd_common(mp-mnt_vnodecovered, cwdi-cwdi_rdir, bp, | buf, MAXBUF / 2, 0, l); Might as well pass NULL for cwdi-cwdi_rdir, since it does the same. But it is less obvious, and haven't advantage. | | -#if 0 | +#if 1 | /* Don't allow more than one mount */ | if (ptyfs_count) | return EBUSY; People did not like that. Didn't like what if 1 or return EBUSY? Ilya.
Re: Enhance ptyfs to handle multiple instances.
On Mar 19, 9:51pm, net...@izyk.ru (Ilya Zykov) wrote:
-- Subject: Re: Enhance ptyfs to handle multiple instances.
| Ok, but bug will stay in the system, temporarily.
No problem, no worse than we have now.
christos
|
| fs/ptyfs/ptyfs_vfsops.c | 16 +++-
| kern/tty_ptm.c |9 -
| 2 files changed, 19 insertions(+), 6 deletions(-)
|
| Ilya.
|
|
| --030108000701050802030304
| Content-Type: text/x-patch;
| name=ptyfs.mi.02.patch
| Content-Transfer-Encoding: 7bit
| Content-Disposition: attachment;
| filename=ptyfs.mi.02.patch
|
| Index: fs/ptyfs/ptyfs_vfsops.c
| ===
| RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
| retrieving revision 1.1.1.1
| diff -u -p -r1.1.1.1 ptyfs_vfsops.c
| --- fs/ptyfs/ptyfs_vfsops.c 4 Mar 2014 18:16:03 - 1.1.1.1
| +++ fs/ptyfs/ptyfs_vfsops.c 19 Mar 2014 17:36:48 -
| @@ -109,14 +109,16 @@ ptyfs__getpath(struct lwp *l, const stru
| buf = malloc(MAXBUF, M_TEMP, M_WAITOK);
| bp = buf + MAXBUF;
| *--bp = '\0';
| - error = getcwd_common(cwdi-cwdi_rdir, rootvnode, bp,
| + error = getcwd_common(mp-mnt_vnodecovered, cwdi-cwdi_rdir, bp,
| buf, MAXBUF / 2, 0, l);
| - if (error) /* XXX */
| + if (error) {/* Mount point is out of rdir */
| + rv = NULL;
| goto out;
| + }
|
| len = strlen(bp);
| if (len sizeof(mp-mnt_stat.f_mntonname)) /* XXX */
| - rv += len;
| + rv += strlen(rv) - len;
| out:
| free(buf, M_TEMP);
| return rv;
| @@ -128,6 +130,7 @@ ptyfs__makename(struct ptm_pty *pt, stru
| {
| struct mount *mp = pt-arg;
| size_t len;
| + const char *np;
|
| switch (ms) {
| case 'p':
| @@ -135,8 +138,11 @@ ptyfs__makename(struct ptm_pty *pt, stru
| len = snprintf(tbuf, bufsiz, /dev/null);
| break;
| case 't':
| - len = snprintf(tbuf, bufsiz, %s/%llu, ptyfs__getpath(l, mp),
| - (unsigned long long)minor(dev));
| + np = ptyfs__getpath(l, mp);
| + if (np == NULL)
| + return EOPNOTSUPP;
| + len = snprintf(tbuf, bufsiz, %s/%llu, np,
| + (unsigned long long)minor(dev));
| break;
| default:
| return EINVAL;
| Index: kern/tty_ptm.c
| ===
| RCS file: /cvsil/nbcur/src/sys/kern/tty_ptm.c,v
| retrieving revision 1.1.1.2
| diff -u -p -r1.1.1.2 tty_ptm.c
| --- kern/tty_ptm.c17 Mar 2014 11:46:10 - 1.1.1.2
| +++ kern/tty_ptm.c19 Mar 2014 17:36:48 -
| @@ -381,7 +381,9 @@ ptmioctl(dev_t dev, u_long cmd, void *da
| goto bad;
|
| /* now, put the indices and names into struct ptmget */
| - return pty_fill_ptmget(l, newdev, cfd, sfd, data);
| + if ((error = pty_fill_ptmget(l, newdev, cfd, sfd, data)) != 0)
| + break; /* goto bad2 */
| + return 0;
| default:
| #ifdef COMPAT_60
| error = compat_60_ptmioctl(dev, cmd, data, flag, l);
| @@ -391,6 +393,11 @@ ptmioctl(dev_t dev, u_long cmd, void *da
| DPRINTF((ptmioctl EINVAL\n));
| return EINVAL;
| }
| +/* bad2: close sfd too */
| + fp = fd_getfile(sfd);
| + if (fp != NULL) {
| + fd_close(sfd);
| + }
| bad:
| fp = fd_getfile(cfd);
| if (fp != NULL) {
|
| --030108000701050802030304--
-- End of excerpt from Ilya Zykov
Re: Enhance ptyfs to handle multiple instances.
| People did not like that.
|
| Didn't like what if 1 or return EBUSY?
The return EBUSY...
Ok, but bug will stay in the system, temporarily.
fs/ptyfs/ptyfs_vfsops.c | 16 +++-
kern/tty_ptm.c |9 -
2 files changed, 19 insertions(+), 6 deletions(-)
Ilya.
Index: fs/ptyfs/ptyfs_vfsops.c
===
RCS file: /cvsil/nbcur/src/sys/fs/ptyfs/ptyfs_vfsops.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 ptyfs_vfsops.c
--- fs/ptyfs/ptyfs_vfsops.c 4 Mar 2014 18:16:03 - 1.1.1.1
+++ fs/ptyfs/ptyfs_vfsops.c 19 Mar 2014 17:36:48 -
@@ -109,14 +109,16 @@ ptyfs__getpath(struct lwp *l, const stru
buf = malloc(MAXBUF, M_TEMP, M_WAITOK);
bp = buf + MAXBUF;
*--bp = '\0';
- error = getcwd_common(cwdi-cwdi_rdir, rootvnode, bp,
+ error = getcwd_common(mp-mnt_vnodecovered, cwdi-cwdi_rdir, bp,
buf, MAXBUF / 2, 0, l);
- if (error) /* XXX */
+ if (error) { /* Mount point is out of rdir */
+ rv = NULL;
goto out;
+ }
len = strlen(bp);
if (len sizeof(mp-mnt_stat.f_mntonname)) /* XXX */
- rv += len;
+ rv += strlen(rv) - len;
out:
free(buf, M_TEMP);
return rv;
@@ -128,6 +130,7 @@ ptyfs__makename(struct ptm_pty *pt, stru
{
struct mount *mp = pt-arg;
size_t len;
+ const char *np;
switch (ms) {
case 'p':
@@ -135,8 +138,11 @@ ptyfs__makename(struct ptm_pty *pt, stru
len = snprintf(tbuf, bufsiz, /dev/null);
break;
case 't':
- len = snprintf(tbuf, bufsiz, %s/%llu, ptyfs__getpath(l, mp),
- (unsigned long long)minor(dev));
+ np = ptyfs__getpath(l, mp);
+ if (np == NULL)
+ return EOPNOTSUPP;
+ len = snprintf(tbuf, bufsiz, %s/%llu, np,
+ (unsigned long long)minor(dev));
break;
default:
return EINVAL;
Index: kern/tty_ptm.c
===
RCS file: /cvsil/nbcur/src/sys/kern/tty_ptm.c,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 tty_ptm.c
--- kern/tty_ptm.c 17 Mar 2014 11:46:10 - 1.1.1.2
+++ kern/tty_ptm.c 19 Mar 2014 17:36:48 -
@@ -381,7 +381,9 @@ ptmioctl(dev_t dev, u_long cmd, void *da
goto bad;
/* now, put the indices and names into struct ptmget */
- return pty_fill_ptmget(l, newdev, cfd, sfd, data);
+ if ((error = pty_fill_ptmget(l, newdev, cfd, sfd, data)) != 0)
+ break; /* goto bad2 */
+ return 0;
default:
#ifdef COMPAT_60
error = compat_60_ptmioctl(dev, cmd, data, flag, l);
@@ -391,6 +393,11 @@ ptmioctl(dev_t dev, u_long cmd, void *da
DPRINTF((ptmioctl EINVAL\n));
return EINVAL;
}
+/* bad2: close sfd too */
+ fp = fd_getfile(sfd);
+ if (fp != NULL) {
+ fd_close(sfd);
+ }
bad:
fp = fd_getfile(cfd);
if (fp != NULL) {
Re: Enhance ptyfs to handle multiple instances.
On Mar 19, 2014, at 1:17 PM, Ilya Zykov net...@izyk.ru wrote: |*--bp = '\0'; | - error = getcwd_common(cwdi-cwdi_rdir, rootvnode, bp, | + error = getcwd_common(mp-mnt_vnodecovered, cwdi-cwdi_rdir, bp, |buf, MAXBUF / 2, 0, l); Might as well pass NULL for cwdi-cwdi_rdir, since it does the same. But it is less obvious, and haven't advantage. | | -#if 0 | +#if 1 |/* Don't allow more than one mount */ |if (ptyfs_count) |return EBUSY; People did not like that. Didn't like what if 1 or return EBUSY”? Speaking only for myself, “#if 1” is a very confusing way to say what a null string expresses much more clearly. paul
Re: Enhance ptyfs to handle multiple instances.
On Mar 14, 12:30pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Enhance ptyfs to handle multiple instances. | Hello! | I desire develop this project. Excellent. | About me. | I am system administrator in little Italy-Russia's firm. I live in Moscow. | OS kernel it's my hobby mainly. | I have free time now and can do this project about 1-2 months. | I have little experience with Linux kernel's tty layer and few accepted patches. | The largest: | https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/tty/tty_buffer.c?id=64325a3be08d364a62ee8f84b2cf86934bc2544a Looks fine to me. | I have few questions about project. | Christos, can I ask you about this? | Please, if anybody has objections or already doing it, tell me know. Nobody is already doing it, and if you have questions, you came to the right place. christos
Re: Enhance ptyfs to handle multiple instances.
| I have few questions about project. | Christos, can I ask you about this? | Please, if anybody has objections or already doing it, tell me know. Nobody is already doing it, and if you have questions, you came to the right place. christos Ok. 1. The main problem and question in this project(IMHO), it's how get access for every instance through one driver ptm[x]. First version. We can do it as well Linux devpts do. Inside every ptyfs we can create not only slave side files, but ptm[x] too for this instance. But who must create(kernel mount function or userspace helper) and what permissions will assign? One more version. We can do many ptm[x] minor numbers(165:0 165:1 for first instance, 165:2 165:3 for second ...) this can be anywhere in fs. But then for every mount we must pass for what instance it's mount doing. We can do it with new mount option instance=#(for example). Every version has advantages and disadvantage. I think first version more clear. What do you think? 2. Mount without new option minstance(for example) must keep old behavior. Is it necessarily? Or every new mount will mount new instance? Thank you. Ilya.
Re: Enhance ptyfs to handle multiple instances.
On Fri, Mar 14, 2014 at 1:29 PM, Ilya Zykov net...@izyk.ru wrote: | I have few questions about project. | Christos, can I ask you about this? | Please, if anybody has objections or already doing it, tell me know. Nobody is already doing it, and if you have questions, you came to the right place. christos Ok. 1. The main problem and question in this project(IMHO), it's how get access for every instance through one driver ptm[x]. First version. We can do it as well Linux devpts do. Inside every ptyfs we can create not only slave side files, but ptm[x] too for this instance. But who must create(kernel mount function or userspace helper) and what permissions will assign? One more version. We can do many ptm[x] minor numbers(165:0 165:1 for first instance, 165:2 165:3 for second ...) this can be anywhere in fs. But then for every mount we must pass for what instance it's mount doing. We can do it with new mount option instance=#(for example). Every version has advantages and disadvantage. I think first version more clear. What do you think? 2. Mount without new option minstance(for example) must keep old behavior. Is it necessarily? Or every new mount will mount new instance? Looking at Linux, it is a little odd, my system does create a ptmx inside /dev/pts/ but there are no read or write permissions set, and everything is actually using the standard /dev/ptmx outside the mount. So the one inside is not much use. All mounted instances (even for namespaces, though have been proposals for device namespaces eg see https://lwn.net/Articles/564854/) are identical, ie have the same slave devices. I can see the advantages in the other options, but I think there may be added complexity, and it might be better to do the first option unless there are strong use cases for multiple instances. Just having multiple mounts is useful. Justin
Re: Enhance ptyfs to handle multiple instances.
On Fri, Mar 14, 2014 at 09:51:12AM -0400, Christos Zoulas wrote: I don't think that putting ptmx inside devpts makes sense. OTOH, we could have multiple ptmx devices with different minor numbers and use that as the differentiating factor for the pty devices. I think that's too complex and probably not worth it (at least in the first pass). Actually, it would simplify things a lot if /dev/ptmx was a symlink to /dev/pts/ptmx. The ptyfs instance could assign a unique minor number per instance and use that to associate the instance with the correct mount point. Joerg
Re: Enhance ptyfs to handle multiple instances.
In article 20140314143532.ga17...@britannica.bec.de, Joerg Sonnenberger jo...@britannica.bec.de wrote: On Fri, Mar 14, 2014 at 09:51:12AM -0400, Christos Zoulas wrote: I don't think that putting ptmx inside devpts makes sense. OTOH, we could have multiple ptmx devices with different minor numbers and use that as the differentiating factor for the pty devices. I think that's too complex and probably not worth it (at least in the first pass). Actually, it would simplify things a lot if /dev/ptmx was a symlink to /dev/pts/ptmx. The ptyfs instance could assign a unique minor number per instance and use that to associate the instance with the correct mount point. Yes, except that ptmx is supposed to work without ptyfs, but I guess that nobody uses it this way anymore. christos
Re: Enhance ptyfs to handle multiple instances.
On 14.03.2014 17:51, Christos Zoulas wrote: On Mar 14, 5:29pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | Ok. | | 1. The main problem and question in this project(IMHO), it's how get access for every instance through one driver ptm[x]. | First version. | We can do it as well Linux devpts do. Inside every ptyfs we can create not only slave side files, | but ptm[x] too for this instance. But who must create(kernel mount function or userspace helper) and what permissions will assign? We first need to decide if disclosing gaps in the pty number is a security issue. If not, it is simple; we just allocate the next free one and we don't care about gaps. I.e. first mount can grab 0,1,2,3,5,6 second mount can grab 4,7,8 etc. It introduces limits. If we care, we can use an indirect mapping. I don't think that we care. I don't think that putting ptmx inside devpts makes sense. We can easy differentiate ptyfs instance. OTOH, we could have multiple ptmx devices with different minor numbers and use that as the differentiating factor for the pty devices. Sorry my tongue-tie, maybe you don't understand me correctly, but this is my (One more version). I think that's too complex and probably not worth it (at least in the first pass). | One more version. | We can do many ptm[x] minor numbers(165:0 165:1 for first instance, 165:2 165:3 for second ...) this can be anywhere in fs. | But then for every mount we must pass for what instance it's mount doing. We can do it with new mount option instance=#(for example). | Every version has advantages and disadvantage. I think first version more clear. What do you think? I think that this is not very desirable because it again introduces limits to the number of ptys per mountpoint. I don't understand how? | 2. Mount without new option minstance(for example) must keep old behavior. Is it necessarily? | Or every new mount will mount new instance? Sure, I don't have a problem with that. An option to mount a copy as opposed to a separate instance is fine. Ok. One remark: mount one instance more than one time useless, because, which mount point must return TIOCPTMGET in this case? Maybe I don't understand fully NetBSD pty layer realization yet. christos
Re: Enhance ptyfs to handle multiple instances.
On Mar 14, 6:49pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | We first need to decide if disclosing gaps in the pty number is a security | issue. If not, it is simple; we just allocate the next free one and we don't | care about gaps. I.e. first mount can grab 0,1,2,3,5,6 second mount can grab | 4,7,8 etc. | | It introduces limits. How so? | I think that this is not very desirable because it again introduces limits | to the number of ptys per mountpoint. | | I don't understand how? if the first mount can only have [0..n-1] the second [n...2*n] etc... | Ok. | One remark: mount one instance more than one time useless, because, which mount point must return TIOCPTMGET in this case? | Maybe I don't understand fully NetBSD pty layer realization yet. If they point to the same device and they are both reachable, it does not matter. If you are inside a chroot, then a reachable one within the root. christos
Re: Enhance ptyfs to handle multiple instances.
On 14.03.2014 18:40, Christos Zoulas wrote: In article 20140314143532.ga17...@britannica.bec.de, Joerg Sonnenberger jo...@britannica.bec.de wrote: On Fri, Mar 14, 2014 at 09:51:12AM -0400, Christos Zoulas wrote: I don't think that putting ptmx inside devpts makes sense. OTOH, we could have multiple ptmx devices with different minor numbers and use that as the differentiating factor for the pty devices. I think that's too complex and probably not worth it (at least in the first pass). Actually, it would simplify things a lot if /dev/ptmx was a symlink to /dev/pts/ptmx. The ptyfs instance could assign a unique minor number per instance and use that to associate the instance with the correct mount point. Yes, except that ptmx is supposed to work without ptyfs, but I guess that nobody uses it this way anymore. I use, for experience :) christos
Re: Enhance ptyfs to handle multiple instances.
if the first mount can only have [0..n-1] the second [n...2*n] etc... 165:0 165:1 is ptm[x] first instance devices. 165:2 165:3 is ptm[x] second instance devices ... Every instance can have [0..N] pts devices. Ilya.
Re: Enhance ptyfs to handle multiple instances.
Hello. I actually do use ptmx(4) without ptyfs, but I would be willing for that to go away in NetBSD-7 or 8. -Brian On Mar 14, 2:40pm, Christos Zoulas wrote: } Subject: Re: Enhance ptyfs to handle multiple instances. } In article 20140314143532.ga17...@britannica.bec.de, } Joerg Sonnenberger jo...@britannica.bec.de wrote: } On Fri, Mar 14, 2014 at 09:51:12AM -0400, Christos Zoulas wrote: } I don't think that putting ptmx inside devpts makes sense. OTOH, we } could have multiple ptmx devices with different minor numbers and use that } as the differentiating factor for the pty devices. I think that's too complex } and probably not worth it (at least in the first pass). } } Actually, it would simplify things a lot if /dev/ptmx was a symlink to } /dev/pts/ptmx. The ptyfs instance could assign a unique minor number per } instance and use that to associate the instance with the correct mount } point. } } Yes, except that ptmx is supposed to work without ptyfs, but I guess that } nobody uses it this way anymore. } } christos } -- End of excerpt from Christos Zoulas
Re: Enhance ptyfs to handle multiple instances.
Date: Fri, 14 Mar 2014 14:40:50 + (UTC) From: chris...@astron.com (Christos Zoulas) In article 20140314143532.ga17...@britannica.bec.de, Joerg Sonnenberger jo...@britannica.bec.de wrote: Actually, it would simplify things a lot if /dev/ptmx was a symlink to /dev/pts/ptmx. The ptyfs instance could assign a unique minor number per instance and use that to associate the instance with the correct mount point. Yes, except that ptmx is supposed to work without ptyfs, but I guess that nobody uses it this way anymore. We could install a symlink at /dev/ptmx pointing to pts/ptmx, and we could install a device node at /dev/pts/ptmx, which gets hidden by the ptyfs mount if you use ptyfs. That way the non-ptyfs case still works and the ptyfs case enables multi-instance goodies.
Re: Enhance ptyfs to handle multiple instances.
On Mar 14, 5:08pm, campbell+netbsd-tech-k...@mumble.net (Taylor R Campbell) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | We could install a symlink at /dev/ptmx pointing to pts/ptmx, and we | could install a device node at /dev/pts/ptmx, which gets hidden by the | ptyfs mount if you use ptyfs. That way the non-ptyfs case still works | and the ptyfs case enables multi-instance goodies. That is the easy part; the harder part is the kernel driver portion. christos
