Source-Changes-HG archive

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

[src/trunk]: src/sys/ufs/ufs Add containment for the cloning devices hack in ...



details:   https://anonhg.NetBSD.org/src/rev/b1f687901492
branches:  trunk
changeset: 379991:b1f687901492
user:      dholland <dholland%NetBSD.org@localhost>
date:      Tue Jun 29 22:40:53 2021 +0000

description:
Add containment for the cloning devices hack in vn_open.

Cloning devices (and also things like /dev/stderr) work by allocating
a struct file, stuffing it in the file table (which is a layer
violation), stuffing the file descriptor number for it in a magic
field of struct lwp (which is gross), and then "failing" with one of
two magic errnos, EDUPFD or EMOVEFD.

Before this commit, all callers of vn_open in the kernel (there are
quite a few) were expected to check for these errors and handle the
situation. Needless to say, none of them except for open() itself did,
resulting in internal negative errnos being returned to userspace.

This hack is fairly deeply rooted and cannot be eliminated all at
once. This commit adds logic to handle the magic errnos inside
vn_open; now on success vn_open returns either a vnode or an integer
file descriptor, along with a flag that says whether the underlying
code requested EDUPFD or EMOVEFD. Callers not prepared to cope with
file descriptors can pass NULL for the extra return values, in which
case if a file descriptor would be produced vn_open fails with
EOPNOTSUPP.

Since I'm rearranging vn_open's signature anyway, stop exposing struct
nameidata. Instead, take three arguments: an optional vnode to use as
the starting point (like openat()), the path, and additional namei
flags to use, restricted to NOCHROOT and TRYEMULROOT. (Other namei
behavior, e.g. NOFOLLOW, can be requested via the open flags.)

This change requires a kernel bump. Ride the one an hour ago.
(That was supposed to be coordinated; did not intend to let an hour
slip by. My fault.)

diffstat:

 external/cddl/osnet/sys/sys/vnode.h |   11 +-
 share/man/man9/errno.9              |    5 +-
 share/man/man9/vnsubr.9             |   82 ++++++++++++++++--
 sys/dev/firmload.c                  |   10 +-
 sys/dev/fss.c                       |   20 ++--
 sys/dev/kloader.c                   |   14 ++-
 sys/dev/vnd.c                       |   42 ++++----
 sys/kern/kern_acct.c                |   29 +++---
 sys/kern/kern_core.c                |   12 +-
 sys/kern/kern_descrip.c             |   50 +++++++----
 sys/kern/kern_ktrace_vfs.c          |   10 +-
 sys/kern/kern_module_vfs.c          |   19 ++--
 sys/kern/subr_exec_fd.c             |   14 +-
 sys/kern/subr_kobj_vfs.c            |   12 +-
 sys/kern/tty_ptm.c                  |    6 +-
 sys/kern/vfs_syscalls.c             |   51 ++++++-----
 sys/kern/vfs_vnops.c                |  153 +++++++++++++++++++++++++----------
 sys/miscfs/fdesc/fdesc_vnops.c      |   16 ++-
 sys/modules/lua/lua.c               |   24 ++--
 sys/sys/filedesc.h                  |    4 +-
 sys/sys/vnode.h                     |    5 +-
 sys/ufs/lfs/ulfs_extattr.c          |   22 ++---
 sys/ufs/lfs/ulfs_quota1.c           |   10 +-
 sys/ufs/ufs/ufs_extattr.c           |   22 ++---
 sys/ufs/ufs/ufs_quota1.c            |   10 +-
 25 files changed, 392 insertions(+), 261 deletions(-)

diffs (truncated from 1668 to 300 lines):

diff -r 460067cb299d -r b1f687901492 external/cddl/osnet/sys/sys/vnode.h
--- a/external/cddl/osnet/sys/sys/vnode.h       Tue Jun 29 22:40:06 2021 +0000
+++ b/external/cddl/osnet/sys/sys/vnode.h       Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vnode.h,v 1.18 2021/04/15 06:59:57 christos Exp $      */
+/*     $NetBSD: vnode.h,v 1.19 2021/06/29 22:40:53 dholland Exp $      */
 
 /*
  * CDDL HEADER START
@@ -239,7 +239,6 @@ zfs_vn_open(const char *pnamep, enum uio
     vnode_t **vpp, enum create crwhy, mode_t umask)
 {
        struct pathbuf *pb;
-       struct nameidata nd;
        int error;
 
        ASSERT(seg == UIO_SYSSPACE);
@@ -247,12 +246,12 @@ zfs_vn_open(const char *pnamep, enum uio
        ASSERT(crwhy == CRCREAT);
        ASSERT(umask == 0);
 
+       filemode |= O_NOFOLLOW;
+
        pb = pathbuf_create(pnamep);
-       NDINIT(&nd, LOOKUP, NOFOLLOW, pb);
-       error = vn_open(&nd, filemode, createmode);
+       error = vn_open(NULL, pb, 0, filemode, createmode, vpp, NULL, NULL);
        if (error == 0) {
-               VOP_UNLOCK(nd.ni_vp, 0);
-               *vpp = nd.ni_vp;
+               VOP_UNLOCK(*vpp, 0);
        }
        pathbuf_destroy(pb);
        return (error);
diff -r 460067cb299d -r b1f687901492 share/man/man9/errno.9
--- a/share/man/man9/errno.9    Tue Jun 29 22:40:06 2021 +0000
+++ b/share/man/man9/errno.9    Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-.\"    $NetBSD: errno.9,v 1.5 2010/03/22 18:58:33 joerg Exp $
+.\"    $NetBSD: errno.9,v 1.6 2021/06/29 22:40:53 dholland Exp $
 .\"
 .\" Copyright (c) 2004 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -97,7 +97,8 @@ to that new file descriptor and return
 .Xr vn_open 9
 takes the file descriptor pointed to by
 .Ar l_dupfd
-and copies it to the file descriptor that the open call will return.
+and arranges for it to be copied to the file descriptor that the open
+call will return.
 .It Er \-6 EMOVEFD Em "Move file descriptor."
 This error is similar to
 .Er EDUPFD
diff -r 460067cb299d -r b1f687901492 share/man/man9/vnsubr.9
--- a/share/man/man9/vnsubr.9   Tue Jun 29 22:40:06 2021 +0000
+++ b/share/man/man9/vnsubr.9   Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-.\"     $NetBSD: vnsubr.9,v 1.47 2019/11/17 11:46:38 wiz Exp $
+.\"     $NetBSD: vnsubr.9,v 1.48 2021/06/29 22:40:53 dholland Exp $
 .\"
 .\" Copyright (c) 2001, 2005, 2006 The NetBSD Foundation, Inc.
 .\" All rights reserved.
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd January 2, 2017
+.Dd June 28, 2021
 .Dt VNSUBR 9
 .Os
 .Sh NAME
@@ -65,7 +65,11 @@
 .Ft void
 .Fn vn_marktext "struct vnode *vp"
 .Ft int
-.Fn vn_open "struct nameidata *ndp" "int fmode" "int cmode"
+.Fo vn_open
+.Fa "struct vnode *at_dvp" "struct pathbuf *pb"
+.Fa "int nmode" "int fmode" "int cmode" 
+.Fa "struct vnode **ret_vp" "bool *ret_domove" "int *ret_fd"
+.Fc
 .Ft int
 .Fn vn_bdev_open "dev_t dev" "struct vnode **vpp" "struct lwp *l"
 .Ft int
@@ -160,25 +164,81 @@ as containing executable code of a runni
 Common code to mark the vnode
 .Fa vp
 as being the text of a running process.
-.It Fn vn_open "ndp" "fmode" "cmode"
+.It Fn vn_open "at_dvp" "pb" "nmode" "fmode" "cmode" "ret_vp" "ret_domove" "ret_fd"
 Common code for vnode open operations.
-The pathname is described in the nameidata pointer (see
-.Xr namei 9 ) .
+The
+.Fa at_dvp
+argument, if not NULL, provides the directory relative paths start
+from, as per
+.Xr openat 2 .
+The
+.Fa pb
+argument gives the pathname.
 The arguments
+.Fa nmode ,
 .Fa fmode
 and
 .Fa cmode
-specify the
+specify additional
+.Xr namei 9
+flags, the
 .Xr open 2
-file mode and the access permissions for creation.
+flags (converted to
+.Dv F*
+from
+.Dv O_*
+form)
+and the access mode (permissions) for creation, respectively.
+The
+.Fa nmode
+argument is restricted to one or perhaps both of the flags
+.Dv TRYEMULROOT
+and
+.Dv NOCHROOT .
+Other
+.Xr name 9
+modes should be selected via
+.Fn fmode .
+The
+.Fa ret_vp ,
+.Fa ret_domove ,
+and
+.Fa ret_fd
+arguments encode the possible return values.
+When called,
 .Fn vn_open
-checks  permissions and invokes the
+checks permissions and invokes the
 .Xr VOP_OPEN 9
 or
 .Xr VOP_CREATE 9
 vnode operations.
-If the operation is successful zero is returned and the vnode is locked,
-otherwise an appropriate error code is returned.
+If the operation is unsuccessful an appropriate error code is returned.
+Otherwise, zero is returned.
+If a vnode is produced, it is returned locked via
+.Fa ret_vp .
+If a file descriptor number is produced instead, the pointer passed via
+.Fa ret_vp
+is NULL, the file descriptor is returned via
+.Fa ret_fd ,
+and the
+.Fa ret_domove
+returns a value that is true if the file descriptor should be moved
+rather than copied.
+These cases correspond to the internal errors
+.Dv EMOVEFD
+and
+.Dv EDUPFD
+respectively.
+See
+.Xr errno 9
+for further information.
+Callers unprepared to handle file descriptors can set
+.Fa ret_fd
+and
+.Fa ret_domove
+to NULL, in which case an operation that would produce a file descriptor
+will instead fail with
+.Dv EOPNOTSUPP .
 .It Fn vn_bdev_open "dev" "vpp" "l"
 Opens a block device by its device number for reading and writing, and
 stores the vnode pointer into
diff -r 460067cb299d -r b1f687901492 sys/dev/firmload.c
--- a/sys/dev/firmload.c        Tue Jun 29 22:40:06 2021 +0000
+++ b/sys/dev/firmload.c        Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: firmload.c,v 1.22 2016/05/30 02:33:49 dholland Exp $   */
+/*     $NetBSD: firmload.c,v 1.23 2021/06/29 22:40:53 dholland Exp $   */
 
 /*-
  * Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: firmload.c,v 1.22 2016/05/30 02:33:49 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: firmload.c,v 1.23 2021/06/29 22:40:53 dholland Exp $");
 
 /*
  * The firmload API provides an interface for device drivers to access
@@ -204,7 +204,6 @@ int
 firmware_open(const char *drvname, const char *imgname, firmware_handle_t *fhp)
 {
        struct pathbuf *pb;
-       struct nameidata nd;
        struct vattr va;
        char *pnbuf, *path, *prefix;
        firmware_handle_t fh;
@@ -236,8 +235,7 @@ firmware_open(const char *drvname, const
                        error = ENOMEM;
                        break;
                }
-               NDINIT(&nd, LOOKUP, FOLLOW | NOCHROOT, pb);
-               error = vn_open(&nd, FREAD, 0);
+               error = vn_open(NULL, pb, NOCHROOT, FREAD, 0, &vp, NULL, NULL);
                pathbuf_destroy(pb);
                if (error == ENOENT) {
                        continue;
@@ -251,8 +249,6 @@ firmware_open(const char *drvname, const
                return (error);
        }
 
-       vp = nd.ni_vp;
-
        error = VOP_GETATTR(vp, &va, kauth_cred_get());
        if (error) {
                VOP_UNLOCK(vp);
diff -r 460067cb299d -r b1f687901492 sys/dev/fss.c
--- a/sys/dev/fss.c     Tue Jun 29 22:40:06 2021 +0000
+++ b/sys/dev/fss.c     Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fss.c,v 1.110 2020/12/26 14:50:50 nia Exp $    */
+/*     $NetBSD: fss.c,v 1.111 2021/06/29 22:40:53 dholland Exp $       */
 
 /*-
  * Copyright (c) 2003 The NetBSD Foundation, Inc.
@@ -36,7 +36,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fss.c,v 1.110 2020/12/26 14:50:50 nia Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fss.c,v 1.111 2021/06/29 22:40:53 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -699,10 +699,9 @@ fss_create_files(struct fss_softc *sc, s
        uint64_t numsec;
        unsigned int secsize;
        struct timespec ts;
-       /* nd -> nd2 to reduce mistakes while updating only some namei calls */
+       /* distinguish lookup 1 from lookup 2 to reduce mistakes */
        struct pathbuf *pb2;
-       struct nameidata nd2;
-       struct vnode *vp;
+       struct vnode *vp, *vp2;
 
        /*
         * Get the mounted file system.
@@ -789,16 +788,17 @@ fss_create_files(struct fss_softc *sc, s
        if (error) {
                return error;
        }
-       NDINIT(&nd2, LOOKUP, FOLLOW, pb2);
-       if ((error = vn_open(&nd2, FREAD|FWRITE, 0)) != 0) {
+       error = vn_open(NULL, pb2, 0, FREAD|FWRITE, 0, &vp2, NULL, NULL);
+       if (error != 0) {
                pathbuf_destroy(pb2);
                return error;
        }
-       VOP_UNLOCK(nd2.ni_vp);
+       VOP_UNLOCK(vp2);
 
-       sc->sc_bs_vp = nd2.ni_vp;
+       sc->sc_bs_vp = vp2;
 
-       if (nd2.ni_vp->v_type != VREG && nd2.ni_vp->v_type != VCHR) {
+       if (vp2->v_type != VREG && vp2->v_type != VCHR) {
+               vrele(vp2);
                pathbuf_destroy(pb2);
                return EINVAL;
        }
diff -r 460067cb299d -r b1f687901492 sys/dev/kloader.c
--- a/sys/dev/kloader.c Tue Jun 29 22:40:06 2021 +0000
+++ b/sys/dev/kloader.c Tue Jun 29 22:40:53 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kloader.c,v 1.28 2020/09/05 16:30:11 riastradh Exp $   */
+/*     $NetBSD: kloader.c,v 1.29 2021/06/29 22:40:53 dholland Exp $    */
 
 /*-
  * Copyright (c) 2001, 2002, 2004 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kloader.c,v 1.28 2020/09/05 16:30:11 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kloader.c,v 1.29 2021/06/29 22:40:53 dholland Exp $");
 
 #include "debug_kloader.h"
 
@@ -586,6 +586,7 @@ kloader_open(const char *filename)
 {
        struct pathbuf *pb;
        struct nameidata nid;
+       struct vnode *vp;
        int error;
 
        pb = pathbuf_create(filename);
@@ -594,8 +595,11 @@ kloader_open(const char *filename)



Home | Main Index | Thread Index | Old Index