Module Name: src Committed By: christos Date: Wed Oct 15 15:00:03 UTC 2014
Modified Files: src/sys/fs/ptyfs: ptyfs_subr.c ptyfs_vfsops.c src/sys/kern: tty_ptm.c src/sys/sys: pty.h Log Message: >From Ilia Zykov: - correct some incorrect comments - add XXX warning - increase security by activating when get the slave - make pty_vn_open() private to tty_ptm.c To generate a diff of this commit: cvs rdiff -u -r1.32 -r1.33 src/sys/fs/ptyfs/ptyfs_subr.c cvs rdiff -u -r1.53 -r1.54 src/sys/fs/ptyfs/ptyfs_vfsops.c cvs rdiff -u -r1.34 -r1.35 src/sys/kern/tty_ptm.c cvs rdiff -u -r1.10 -r1.11 src/sys/sys/pty.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/fs/ptyfs/ptyfs_subr.c diff -u src/sys/fs/ptyfs/ptyfs_subr.c:1.32 src/sys/fs/ptyfs/ptyfs_subr.c:1.33 --- src/sys/fs/ptyfs/ptyfs_subr.c:1.32 Sat Aug 16 03:22:30 2014 +++ src/sys/fs/ptyfs/ptyfs_subr.c Wed Oct 15 11:00:03 2014 @@ -1,4 +1,4 @@ -/* $NetBSD: ptyfs_subr.c,v 1.32 2014/08/16 07:22:30 hannken Exp $ */ +/* $NetBSD: ptyfs_subr.c,v 1.33 2014/10/15 15:00:03 christos Exp $ */ /* * Copyright (c) 1993 @@ -73,7 +73,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.32 2014/08/16 07:22:30 hannken Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ptyfs_subr.c,v 1.33 2014/10/15 15:00:03 christos Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -177,6 +177,12 @@ ptyfs_get_node(ptyfstype type, int pty) pp->ptyfs_atime = pp->ptyfs_ctime; pp->ptyfs_flags = 0; mutex_enter(&ptyfs_hashlock); + /* + * XXX We have minimum race condition when opening master side + * first time, if other threads through other mount points, trying + * opening the same device. As follow we have little chance have + * unused list entries. + */ SLIST_INSERT_HEAD(ppp, pp, ptyfs_hash); mutex_exit(&ptyfs_hashlock); return pp; Index: src/sys/fs/ptyfs/ptyfs_vfsops.c diff -u src/sys/fs/ptyfs/ptyfs_vfsops.c:1.53 src/sys/fs/ptyfs/ptyfs_vfsops.c:1.54 --- src/sys/fs/ptyfs/ptyfs_vfsops.c:1.53 Fri Aug 15 09:40:39 2014 +++ src/sys/fs/ptyfs/ptyfs_vfsops.c Wed Oct 15 11:00:03 2014 @@ -1,4 +1,4 @@ -/* $NetBSD: ptyfs_vfsops.c,v 1.53 2014/08/15 13:40:39 hannken Exp $ */ +/* $NetBSD: ptyfs_vfsops.c,v 1.54 2014/10/15 15:00:03 christos Exp $ */ /* * Copyright (c) 1992, 1993, 1995 @@ -38,7 +38,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ptyfs_vfsops.c,v 1.53 2014/08/15 13:40:39 hannken Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ptyfs_vfsops.c,v 1.54 2014/10/15 15:00:03 christos Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -217,7 +217,8 @@ ptyfs__allocvp(struct mount *mp, struct *vpp = NULL; return error; } - if (type == PTYFSptc) + /* Activate node only after we have grabbed device. */ + if (type == PTYFSpts) ptyfs_set_active(mp, minor(dev)); return 0; } @@ -415,7 +416,8 @@ ptyfs_sync(struct mount *mp, int waitfor /* * Initialize this vnode / ptynode pair. - * Caller assures no other thread will try to load this node. + * Only for the slave side of a pty, caller assures + * no other thread will try to load this node. */ int ptyfs_loadvnode(struct mount *mp, struct vnode *vp, Index: src/sys/kern/tty_ptm.c diff -u src/sys/kern/tty_ptm.c:1.34 src/sys/kern/tty_ptm.c:1.35 --- src/sys/kern/tty_ptm.c:1.34 Fri Sep 5 05:20:59 2014 +++ src/sys/kern/tty_ptm.c Wed Oct 15 11:00:03 2014 @@ -1,4 +1,4 @@ -/* $NetBSD: tty_ptm.c,v 1.34 2014/09/05 09:20:59 matt Exp $ */ +/* $NetBSD: tty_ptm.c,v 1.35 2014/10/15 15:00:03 christos Exp $ */ /*- * Copyright (c) 2004 The NetBSD Foundation, Inc. @@ -27,7 +27,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: tty_ptm.c,v 1.34 2014/09/05 09:20:59 matt Exp $"); +__KERNEL_RCSID(0, "$NetBSD: tty_ptm.c,v 1.35 2014/10/15 15:00:03 christos Exp $"); #include "opt_compat_netbsd.h" #include "opt_ptm.h" @@ -87,6 +87,7 @@ int pts_major, ptc_major; static dev_t pty_getfree(void); static int pty_alloc_master(struct lwp *, int *, dev_t *, struct mount *); static int pty_alloc_slave(struct lwp *, int *, dev_t, struct mount *); +static int pty_vn_open(struct vnode *, struct lwp *); void ptmattach(int); @@ -174,22 +175,6 @@ 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; Index: src/sys/sys/pty.h diff -u src/sys/sys/pty.h:1.10 src/sys/sys/pty.h:1.11 --- src/sys/sys/pty.h:1.10 Fri Apr 4 14:11:58 2014 +++ src/sys/sys/pty.h Wed Oct 15 11:00:03 2014 @@ -1,4 +1,4 @@ -/* $NetBSD: pty.h,v 1.10 2014/04/04 18:11:58 christos Exp $ */ +/* $NetBSD: pty.h,v 1.11 2014/10/15 15:00:03 christos Exp $ */ /*- * Copyright (c) 2004 The NetBSD Foundation, Inc. @@ -39,14 +39,13 @@ void ptmattach(int); int pty_fill_ptmget(struct lwp *, dev_t, int, int, void *, struct mount *); int pty_grant_slave(struct lwp *, dev_t, struct mount *); dev_t pty_makedev(char, int); -int pty_vn_open(struct vnode *, struct lwp *); struct ptm_pty *pty_sethandler(struct ptm_pty *); int pty_getmp(struct lwp *, struct mount **); /* * Ptm_pty is used for switch ptm{x} driver between BSDPTY, PTYFS. * Functions' argument (struct mount *) is used only PTYFS, - * in the case BSDPTY can be NULL, and arg must be NULL. + * in the case BSDPTY can be NULL. */ struct ptm_pty { int (*allocvp)(struct mount *, struct lwp *, struct vnode **, dev_t,