Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 2 Apr 2021 12:40:53 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 76b1b5ce6d81 - main - nullfs: protect against user creating inconsistent state
Message-ID:  <202104021240.132CerGr055353@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=76b1b5ce6d81f66b09be8a20aecd064b65fd6b50

commit 76b1b5ce6d81f66b09be8a20aecd064b65fd6b50
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-04-01 17:42:14 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2021-04-02 12:40:25 +0000

    nullfs: protect against user creating inconsistent state
    
    The VFS conventions is that VOP_LOOKUP() methods do not need to handle
    ISDOTDOT lookups for VV_ROOT vnodes (since they cannot, after all).  Nullfs
    bypasses VOP_LOOKUP() to lower filesystem, and there, due to user actions,
    it is possible to get into situation where
    - upper vnode does not have VV_ROOT set
    - lower vnode is root
    - ISDOTDOT is requested
    User just needs to nullfs-mount non-root of some filesystem, and then move
    some directory under mount, out of mount, using lower filesystem.
    
    In this case, nullfs cannot do much, but we still should and can ensure
    internal kernel structures are consistent.  Avoid ISDOTDOT lookup forwarding
    when VV_ROOT is set on lower dvp, return somewhat arbitrary ENOENT.
    
    PR:     253593
    Reported by:    Gregor Koscak <elogin41@gmail.com>
    Test by:        Patrick Sullivan <sulli00777@gmail.com>
    Tested by:      pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
---
 sys/fs/nullfs/null_vnops.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c
index bc0f5cdb7801..5bf470897c08 100644
--- a/sys/fs/nullfs/null_vnops.c
+++ b/sys/fs/nullfs/null_vnops.c
@@ -389,10 +389,21 @@ null_lookup(struct vop_lookup_args *ap)
 	 */
 	ldvp = NULLVPTOLOWERVP(dvp);
 	vp = lvp = NULL;
-	KASSERT((ldvp->v_vflag & VV_ROOT) == 0 ||
-	    ((dvp->v_vflag & VV_ROOT) != 0 && (flags & ISDOTDOT) == 0),
-	    ("ldvp %p fl %#x dvp %p fl %#x flags %#x", ldvp, ldvp->v_vflag,
-	     dvp, dvp->v_vflag, flags));
+
+	/*
+	 * Renames in the lower mounts might create an inconsistent
+	 * configuration where lower vnode is moved out of the
+	 * directory tree remounted by our null mount.  Do not try to
+	 * handle it fancy, just avoid VOP_LOOKUP() with DOTDOT name
+	 * which cannot be handled by VOP, at least passing over lower
+	 * root.
+	 */
+	if ((ldvp->v_vflag & VV_ROOT) != 0 && (flags & ISDOTDOT) != 0) {
+		KASSERT((dvp->v_vflag & VV_ROOT) == 0,
+		    ("ldvp %p fl %#x dvp %p fl %#x flags %#x",
+		    ldvp, ldvp->v_vflag, dvp, dvp->v_vflag, flags));
+		return (ENOENT);
+	}
 
 	/*
 	 * Hold ldvp.  The reference on it, owned by dvp, is lost in



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104021240.132CerGr055353>