Re: Enhance ptyfs to handle multiple instances.
On Apr 4, 8:16pm, net...@izyk.ru (Ilya Zykov) wrote: -- Subject: Re: Enhance ptyfs to handle multiple instances. | Small test. thanks, I tested with script(1)... christos
Re: Enhance ptyfs to handle multiple instances.
Small test.
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
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 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.
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 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? > Yes.
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: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.
> > 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 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: MBUFTRACE panic (KASSERT off < percpu_nextoff)
On Thu, Apr 03, 2014 at 05:51:38PM +0200, Manuel Bouyer wrote: > Hello, > trying to debug > http://mail-index.netbsd.org/current-users/2014/03/26/msg024535.html > I built a kernel with MBUFTRACE (this is netbsd-6 of december). > I got this almost immediatly: > > panic: kernel diagnostic assertion "off < percpu_nextoff" failed: file > "../../.. /../kern/subr_percpu.c", line 76 > cpu0: Begin traceback... > kern_assert() at netbsd:kern_assert+0x48 > percpu_offset() at netbsd:percpu_offset+0x4b > percpu_getref() at netbsd:percpu_getref+0x40 > m_claim() at netbsd:m_claim+0x44 > m_claimm() at netbsd:m_claimm+0x1d > ether_input() at netbsd:ether_input+0x33 > carp_input() at netbsd:carp_input+0x7f I found that carp(4) doens't do the proper initialisations for MBUFTRACE. So this is fixed; now back to the mbuf leak issue ... -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
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
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.
