[src/trunk]: src/sys Add a new namei flag NONEXCLHACK for open with O_CREAT a...

[dholland <dholland%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/fd60c55a30cc
branches:  trunk
changeset: 379719:fd60c55a30cc
user:      dholland <dholland%NetBSD.org@localhost>
date:      Wed Jun 16 01:51:57 2021 +0000

description:
Add a new namei flag NONEXCLHACK for open with O_CREAT and not O_EXCL.

This case needs to be distinguished from the other CREATE operations
because it is supposed to successfully return (and open) the target if
it exists. In the case where that target is the root, or a mount
point, such that there's no parent dir, "real" CREATE operations fail,
but O_CREAT without O_EXCL needs to succeed.

So (a) add the flag, (b) test for it in namei in the situation
described above, (c) set it in open under the appropriate
circumstances, and (d) because this can result in namei returning
ni_dvp of NULL, cope with that case.

Should get into -9 and maybe even -8, because it was prompted by
issues with 3rd-party code. The use of a flag (vs. adding an
additional nameiop, which would be more appropriate) was deliberate to
make the patch small and noninvasive.

diffstat:

 sys/kern/vfs_lookup.c |  29 ++++++++++++++++++++++++++---
 sys/kern/vfs_vnops.c  |  29 +++++++++++++++++++++++++----
 sys/sys/namei.src     |   5 +++--
 3 files changed, 54 insertions(+), 9 deletions(-)

diffs (143 lines):

diff -r 39c97a263ef6 -r fd60c55a30cc sys/kern/vfs_lookup.c
--- a/sys/kern/vfs_lookup.c     Wed Jun 16 00:56:16 2021 +0000
+++ b/sys/kern/vfs_lookup.c     Wed Jun 16 01:51:57 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_lookup.c,v 1.225 2020/12/29 22:13:40 chs Exp $     */
+/*     $NetBSD: vfs_lookup.c,v 1.226 2021/06/16 01:51:57 dholland Exp $        */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.225 2020/12/29 22:13:40 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c,v 1.226 2021/06/16 01:51:57 dholland Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_magiclinks.h"
@@ -1802,10 +1802,33 @@ namei_oneroot(struct namei_state *state,
                 * a CREATE, DELETE, or RENAME), and we don't have one
                 * (because this is the root directory, or we crossed
                 * a mount point), then we must fail.
+                *
+                * 20210604 dholland when NONEXCLHACK is set (open
+                * with O_CREAT but not O_EXCL) skip this logic. Since
+                * we have a foundobj, open will not be creating, so
+                * it doesn't actually need or use the searchdir, so
+                * it's ok to return it even if it's on a different
+                * volume, and it's also ok to return NULL; by setting
+                * NONEXCLHACK the open code promises to cope with
+                * those cases correctly. (That is, it should do what
+                * it would do anyway, that is, just release the
+                * searchdir, except not crash if it's null.) This is
+                * needed because otherwise opening mountpoints with
+                * O_CREAT but not O_EXCL fails... which is a silly
+                * thing to do but ought to work. (This whole issue
+                * came to light because 3rd party code wanted to open
+                * certain procfs nodes with O_CREAT for some 3rd
+                * party reason, and it failed.)
+                *
+                * Note that NONEXCLHACK is properly a different
+                * nameiop (it is partway between LOOKUP and CREATE)
+                * but it was stuffed in as a flag instead to make the
+                * resulting patch less invasive for pullup. Blah.
                 */
                if (cnp->cn_nameiop != LOOKUP &&
                    (searchdir == NULL ||
-                    searchdir->v_mount != foundobj->v_mount)) {
+                    searchdir->v_mount != foundobj->v_mount) &&
+                   (cnp->cn_flags & NONEXCLHACK) == 0) {
                        if (searchdir) {
                                if (searchdir_locked) {
                                        vput(searchdir);
diff -r 39c97a263ef6 -r fd60c55a30cc sys/kern/vfs_vnops.c
--- a/sys/kern/vfs_vnops.c      Wed Jun 16 00:56:16 2021 +0000
+++ b/sys/kern/vfs_vnops.c      Wed Jun 16 01:51:57 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_vnops.c,v 1.214 2020/11/09 18:09:02 chs Exp $      */
+/*     $NetBSD: vfs_vnops.c,v 1.215 2021/06/16 01:51:57 dholland Exp $ */
 
 /*-
  * Copyright (c) 2009 The NetBSD Foundation, Inc.
@@ -66,7 +66,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_vnops.c,v 1.214 2020/11/09 18:09:02 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_vnops.c,v 1.215 2021/06/16 01:51:57 dholland Exp $");
 
 #include "veriexec.h"
 
@@ -161,6 +161,8 @@ vn_open(struct nameidata *ndp, int fmode
                if ((fmode & O_EXCL) == 0 &&
                    ((fmode & O_NOFOLLOW) == 0))
                        ndp->ni_cnd.cn_flags |= FOLLOW;
+               if ((fmode & O_EXCL) == 0)
+                       ndp->ni_cnd.cn_flags |= NONEXCLHACK;
        } else {
                ndp->ni_cnd.cn_nameiop = LOOKUP;
                ndp->ni_cnd.cn_flags |= LOCKLEAF;
@@ -183,7 +185,12 @@ vn_open(struct nameidata *ndp, int fmode
        error = veriexec_openchk(l, ndp->ni_vp, pathstring, fmode);
        if (error) {
                /* We have to release the locks ourselves */
-               if (fmode & O_CREAT) {
+               /*
+                * 20210604 dholland passing NONEXCLHACK means we can
+                * get ni_dvp == NULL back if ni_vp exists, and we should
+                * treat that like the non-O_CREAT case.
+                */
+               if ((fmode & O_CREAT) != 0 && ndp->ni_dvp != NULL) {
                        if (vp == NULL) {
                                vput(ndp->ni_dvp);
                        } else {
@@ -202,7 +209,10 @@ vn_open(struct nameidata *ndp, int fmode
        }
 #endif /* NVERIEXEC > 0 */
 
-       if (fmode & O_CREAT) {
+       /*
+        * 20210604 dholland ditto
+        */
+       if ((fmode & O_CREAT) != 0 && ndp->ni_dvp != NULL) {
                if (ndp->ni_vp == NULL) {
                        vattr_null(&va);
                        va.va_type = VREG;
@@ -233,6 +243,17 @@ vn_open(struct nameidata *ndp, int fmode
                        }
                        fmode &= ~O_CREAT;
                }
+       } else if ((fmode & O_CREAT) != 0) {
+               /*
+                * 20210606 dholland passing NONEXCLHACK means this
+                * case exists; it is the same as the following one
+                * but also needs to do things in the second (exists)
+                * half of the following block. (Besides handle
+                * ni_dvp, anyway.)
+                */
+               vp = ndp->ni_vp;
+               KASSERT((fmode & O_EXCL) == 0);
+               fmode &= ~O_CREAT;
        } else {
                vp = ndp->ni_vp;
        }
diff -r 39c97a263ef6 -r fd60c55a30cc sys/sys/namei.src
--- a/sys/sys/namei.src Wed Jun 16 00:56:16 2021 +0000
+++ b/sys/sys/namei.src Wed Jun 16 01:51:57 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: namei.src,v 1.58 2020/05/30 20:16:14 ad Exp $  */
+/*     $NetBSD: namei.src,v 1.59 2021/06/16 01:51:57 dholland Exp $    */
 
 /*
  * Copyright (c) 1985, 1989, 1991, 1993
@@ -154,7 +154,8 @@ NAMEIFL     EMULROOTSET     0x00000080      /* emulat
                                           in ni_erootdir */
 NAMEIFL        LOCKSHARED      0x00000100      /* want shared locks if possible */
 NAMEIFL        NOCHROOT        0x01000000      /* no chroot on abs path lookups */
-NAMEIFL        MODMASK         0x010001fc      /* mask of operational modifiers */
+NAMEIFL        NONEXCLHACK     0x02000000      /* open wwith O_CREAT but not O_EXCL */
+NAMEIFL        MODMASK         0x030001fc      /* mask of operational modifiers */
 /*
  * Namei parameter descriptors.
  */