Source-Changes-HG archive

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

[src/ad-namecache]: src/sys - Add a LOCKSHARED flag to namei (matching FreeBS...



details:   https://anonhg.NetBSD.org/src/rev/d2dfdfcaebb4
branches:  ad-namecache
changeset: 1025031:d2dfdfcaebb4
user:      ad <ad%NetBSD.org@localhost>
date:      Sun Jan 19 21:19:25 2020 +0000

description:
- Add a LOCKSHARED flag to namei (matching FreeBSD) indicating that we want
  the leaf locked with LK_SHARED.

- Add an IMNT_SHRLOOKUP flag to struct mount indicating that the file
  system can do VOP_LOOKUP() with an shared lock.  If it encounters
  something tricky, VOP_LOOKUP() is free to return ENOLCK and namei() will
  retry the lookup with an exclusive lock.  If the file system has this flag
  set, namei() will try with shared locks for all of the "read only"
  lookups, i.e. nameiop=LOOKUP or !ISLASTCN.

- vfs_getcwd: only take vnode locks when really needed, take shared locks if
  possible, and where the namecache has identify info for the directories,
  do it all in the namecache.

- vfs_lookup: when crossing mountpoints take only a shared lock on the
  covered vnode; don't need anything else.

diffstat:

 sys/kern/vfs_cache.c  |   64 ++++++++++++++--
 sys/kern/vfs_getcwd.c |  189 +++++++++++++++++++++----------------------------
 sys/kern/vfs_lookup.c |  123 ++++++++++++++++++++++---------
 sys/sys/fstypes.h     |    4 +-
 sys/sys/namei.src     |   16 ++-
 5 files changed, 233 insertions(+), 163 deletions(-)

diffs (truncated from 804 to 300 lines):

diff -r a99da97519cb -r d2dfdfcaebb4 sys/kern/vfs_cache.c
--- a/sys/kern/vfs_cache.c      Sun Jan 19 21:10:59 2020 +0000
+++ b/sys/kern/vfs_cache.c      Sun Jan 19 21:19:25 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_cache.c,v 1.126.2.8 2020/01/18 17:16:20 ad Exp $   */
+/*     $NetBSD: vfs_cache.c,v 1.126.2.9 2020/01/19 21:19:25 ad Exp $   */
 
 /*-
  * Copyright (c) 2008, 2019, 2020 The NetBSD Foundation, Inc.
@@ -148,7 +148,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.126.2.8 2020/01/18 17:16:20 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.126.2.9 2020/01/19 21:19:25 ad Exp $");
 
 #define __NAMECACHE_PRIVATE
 #ifdef _KERNEL_OPT
@@ -326,11 +326,15 @@
        KASSERT(nlen <= USHRT_MAX);
 
        key = hash32_buf(name, nlen, HASH32_STR_INIT);
-       return (key << 16) | nlen;
+       return (key << 32) | nlen;
 }
 
 /*
- * Like memcmp() but tuned for small strings of equal length.
+ * Like bcmp() but tuned for the use case here which is:
+ *
+ * - always of equal length both sides
+ * - almost always the same string both sides
+ * - small strings
  */
 static inline int
 cache_namecmp(struct namecache *nc, const char *name, size_t namelen)
@@ -743,7 +747,8 @@
  * Returns 0 on success, -1 on cache miss, positive errno on failure.
  */
 int
-cache_revlookup(struct vnode *vp, struct vnode **dvpp, char **bpp, char *bufp)
+cache_revlookup(struct vnode *vp, struct vnode **dvpp, char **bpp, char *bufp,
+    bool checkaccess, int perms)
 {
        struct nchnode *nn = VNODE_TO_VIMPL(vp)->vi_ncache;
        struct namecache *nc;
@@ -757,6 +762,27 @@
                goto out;
 
        rw_enter(&nn->nn_listlock, RW_READER);
+       if (checkaccess) {
+               /*
+                * Check if the user is allowed to see.  NOTE: this is
+                * checking for access on the "wrong" directory.  getcwd()
+                * wants to see that there is access on every component
+                * along the way, not that there is access to any individual
+                * component.
+                */
+               KASSERT(nn->nn_mode != VNOVAL && nn->nn_uid != VNOVAL &&
+                   nn->nn_gid != VNOVAL);
+               error = kauth_authorize_vnode(curlwp->l_cred,
+                   KAUTH_ACCESS_ACTION(VEXEC, vp->v_type, nn->nn_mode &
+                   ALLPERMS), vp, NULL, genfs_can_access(vp->v_type,
+                   nn->nn_mode & ALLPERMS, nn->nn_uid, nn->nn_gid,
+                   perms, curlwp->l_cred));
+                   if (error != 0) {
+                       rw_exit(&nn->nn_listlock);
+                       COUNT(ncs_denied);
+                       return EACCES;
+               }
+       }
        TAILQ_FOREACH(nc, &nn->nn_list, nc_list) {
                KASSERT(nc->nc_nn == nn);
                KASSERT(nc->nc_dnn != NULL);
@@ -931,12 +957,14 @@
 
        if (dvp->v_type == VDIR) {
                rw_enter(&nn->nn_lock, RW_WRITER);
+               rw_enter(&nn->nn_listlock, RW_WRITER);
                KASSERT(nn->nn_mode == VNOVAL);
                KASSERT(nn->nn_uid == VNOVAL);
                KASSERT(nn->nn_gid == VNOVAL);
                nn->nn_mode = mode;
                nn->nn_uid = uid;
                nn->nn_gid = gid;
+               rw_exit(&nn->nn_listlock);
                rw_exit(&nn->nn_lock);
        }
 }
@@ -953,16 +981,30 @@
 
        if (dvp->v_type == VDIR) {
                rw_enter(&nn->nn_lock, RW_WRITER);
+               rw_enter(&nn->nn_listlock, RW_WRITER);
                if (nn->nn_mode != VNOVAL) {
                        nn->nn_mode = mode;
                        nn->nn_uid = uid;
                        nn->nn_gid = gid;
                }
+               rw_exit(&nn->nn_listlock);
                rw_exit(&nn->nn_lock);
        }
 }
 
 /*
+ * Return true if we have identity for the given vnode.
+ */
+bool
+cache_have_id(struct vnode *dvp)
+{
+       struct nchnode *nn = VNODE_TO_VIMPL(dvp)->vi_ncache;
+
+       /* Unlocked check.  Only goes VNOVAL -> valid, never back. */
+       return nn->nn_mode != VNOVAL;
+}
+
+/*
  * Name cache initialization, from vfs_init() when the system is booting.
  */
 void
@@ -1259,7 +1301,7 @@
 cache_reclaim(void)
 {
        struct namecache *nc;
-       struct nchnode *nn;
+       struct nchnode *dnn;
        int toscan, total;
 
        /* Scan up to a preset maxium number of entries. */
@@ -1276,9 +1318,9 @@
                if (nc == NULL) {
                        break;
                }
-               nn = nc->nc_nn;
+               dnn = nc->nc_dnn;
                KASSERT(nc->nc_lrulist == LRU_INACTIVE);
-               KASSERT(nn != NULL);
+               KASSERT(dnn != NULL);
 
                /*
                 * Locking in the wrong direction.  If we can't get the
@@ -1286,7 +1328,7 @@
                 * cause problems for the next guy in here, so send the
                 * entry to the back of the list.
                 */
-               if (!rw_tryenter(&nn->nn_lock, RW_WRITER)) {
+               if (!rw_tryenter(&dnn->nn_lock, RW_WRITER)) {
                        TAILQ_REMOVE(&cache_lru.list[LRU_INACTIVE],
                            nc, nc_lru);
                        TAILQ_INSERT_TAIL(&cache_lru.list[LRU_INACTIVE],
@@ -1303,7 +1345,7 @@
                 */
                mutex_exit(&cache_lru_lock);
                cache_remove(nc, true);
-               rw_exit(&nn->nn_lock);
+               rw_exit(&dnn->nn_lock);
                mutex_enter(&cache_lru_lock);
        }
        mutex_exit(&cache_lru_lock);
@@ -1388,8 +1430,8 @@
 void
 namecache_print(struct vnode *vp, void (*pr)(const char *, ...))
 {
+       struct nchnode *dnn = NULL;
        struct namecache *nc;
-       struct nchnode *dnn;
        enum cache_lru_id id;
 
        for (id = 0; id < LRU_COUNT; id++) {
diff -r a99da97519cb -r d2dfdfcaebb4 sys/kern/vfs_getcwd.c
--- a/sys/kern/vfs_getcwd.c     Sun Jan 19 21:10:59 2020 +0000
+++ b/sys/kern/vfs_getcwd.c     Sun Jan 19 21:19:25 2020 +0000
@@ -1,7 +1,7 @@
-/* $NetBSD: vfs_getcwd.c,v 1.53.2.2 2020/01/17 21:54:27 ad Exp $ */
+/* $NetBSD: vfs_getcwd.c,v 1.53.2.3 2020/01/19 21:19:25 ad Exp $ */
 
 /*-
- * Copyright (c) 1999 The NetBSD Foundation, Inc.
+ * Copyright (c) 1999, 2020 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_getcwd.c,v 1.53.2.2 2020/01/17 21:54:27 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_getcwd.c,v 1.53.2.3 2020/01/19 21:19:25 ad Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -87,7 +87,7 @@
  * On exit, *uvpp is either NULL or is a locked vnode reference.
  */
 static int
-getcwd_scandir(struct vnode **lvpp, struct vnode **uvpp, char **bpp,
+getcwd_scandir(struct vnode *lvp, struct vnode **uvpp, char **bpp,
     char *bufp, struct lwp *l)
 {
        int     error = 0;
@@ -101,14 +101,13 @@
        ino_t   fileno;
        struct vattr va;
        struct vnode *uvp = NULL;
-       struct vnode *lvp = *lvpp;
        kauth_cred_t cred = l->l_cred;
        struct componentname cn;
        int len, reclen;
        tries = 0;
 
-       /* Upgrade to exclusive for UFS VOP_GETATTR (itimes) & VOP_LOOKUP. */
-       vn_lock(lvp, LK_UPGRADE | LK_RETRY);
+       /* Need exclusive for UFS VOP_GETATTR (itimes) & VOP_LOOKUP. */
+       KASSERT(VOP_ISLOCKED(lvp) == LK_EXCLUSIVE);
 
        /*
         * If we want the filename, get some info we need while the
@@ -117,8 +116,7 @@
        if (bufp != NULL) {
                error = VOP_GETATTR(lvp, &va, cred);
                if (error) {
-                       vput(lvp);
-                       *lvpp = NULL;
+                       VOP_UNLOCK(lvp);
                        *uvpp = NULL;
                        return error;
                }
@@ -137,24 +135,14 @@
 
        /* At this point, lvp is locked  */
        error = VOP_LOOKUP(lvp, uvpp, &cn);
-       vput(lvp);
+       VOP_UNLOCK(lvp);
        if (error) {
-               *lvpp = NULL;
                *uvpp = NULL;
                return error;
        }
        uvp = *uvpp;
-       /* Now lvp is unlocked, try to lock uvp */
-       error = vn_lock(uvp, LK_SHARED);
-       if (error) {
-               *lvpp = NULL;
-               *uvpp = NULL;
-               return error;
-       }
-
        /* If we don't care about the pathname, we're done */
        if (bufp == NULL) {
-               *lvpp = NULL;
                return 0;
        }
 
@@ -166,6 +154,14 @@
                dirbuflen = va.va_blocksize;
        dirbuf = kmem_alloc(dirbuflen, KM_SLEEP);
 
+       /* Now lvp is unlocked, try to lock uvp */
+       error = vn_lock(uvp, LK_SHARED);
+       if (error) {
+               vrele(uvp);
+               *uvpp = NULL;
+               return error;
+       }
+
 #if 0
 unionread:
 #endif
@@ -264,66 +260,14 @@
        error = ENOENT;
 
 out:
-       *lvpp = NULL;
+       VOP_UNLOCK(uvp);
        kmem_free(dirbuf, dirbuflen);
        return error;
 }
 
 /*
- * Look in the vnode-to-name reverse cache to see if
- * we can find things the easy way.
- *
- * XXX vget failure path is untested.
- *
- * On entry, *lvpp is a locked vnode reference.
- * On exit, one of the following is the case:
- *     0) Both *lvpp and *uvpp are NULL and failure is returned.
- *     1) *uvpp is NULL, *lvpp remains locked and -1 is returned (cache miss)
- *     2) *uvpp is a locked vnode reference, *lvpp is vput and NULL'ed
- *        and 0 is returned (cache hit)
- */
-
-static int
-getcwd_getcache(struct vnode **lvpp, struct vnode **uvpp, char **bpp,
-    char *bufp)
-{
-       struct vnode *lvp, *uvp = NULL;
-       int error;
-
-       lvp = *lvpp;



Home | Main Index | Thread Index | Old Index