The branch stable/14 has been updated by markj:
URL:
https://cgit.FreeBSD.org/src/commit/?id=3feafab4a34c95209cd4fc3e6224c324efc056f3
commit 3feafab4a34c95209cd4fc3e6224c324efc056f3
Author: Mark Johnston <ma...@freebsd.org>
AuthorDate: 2025-05-23 12:52:24 +0000
Commit: Mark Johnston <ma...@freebsd.org>
CommitDate: 2025-06-30 14:19:30 +0000
namei: Make stackable filesystems check harder for jail roots
Suppose a process has its cwd pointing to a nullfs directory, where the
lower directory is also visible in the jail's filesystem namespace.
Suppose that the lower directory vnode is moved out from under the
nullfs mount. The nullfs vnode still shadows the lower vnode, and
dotdot lookups relative to that directory will instantiate new nullfs
vnodes outside of the nullfs mountpoint, effectively shadowing the lower
filesystem.
This phenomenon can be abused to escape a chroot, since the nullfs
vnodes instantiated by these dotdot lookups defeat the root vnode check
in vfs_lookup(), which uses vnode pointer equality to test for the
process root.
Fix this by extending nullfs and unionfs to perform the same check,
exploiting the fact that the passed componentname is embedded in a
nameidata structure to avoid changing the VOP_LOOKUP interface. That
is, add a flag to indicate that containerof can be used to get the full
nameidata structure, and perform the root vnode check on the lower vnode
when performing a dotdot lookup.
PR: 262180
Reviewed by: olce, kib
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D50418
(cherry picked from commit 7587f6d4840f8d363e457cddc14c184cf1fe7cc1)
---
sys/fs/nullfs/null_vnops.c | 28 ++++++++++++++++++----------
sys/fs/unionfs/union_vnops.c | 21 +++++++++++++++++++++
sys/kern/vfs_cache.c | 11 +----------
sys/kern/vfs_lookup.c | 41 ++++++++++++++++++++++++++++++-----------
sys/sys/namei.h | 5 ++++-
5 files changed, 74 insertions(+), 32 deletions(-)
diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c
index f2c426d41f61..41915da7f13c 100644
--- a/sys/fs/nullfs/null_vnops.c
+++ b/sys/fs/nullfs/null_vnops.c
@@ -407,17 +407,25 @@ null_lookup(struct vop_lookup_args *ap)
/*
* 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.
+ * 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 the VOP.
*/
- 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 %#jx",
- ldvp, ldvp->v_vflag, dvp, dvp->v_vflag, (uintmax_t)flags));
- return (ENOENT);
+ if ((flags & ISDOTDOT) != 0) {
+ struct nameidata *ndp;
+
+ if ((ldvp->v_vflag & VV_ROOT) != 0) {
+ KASSERT((dvp->v_vflag & VV_ROOT) == 0,
+ ("ldvp %p fl %#x dvp %p fl %#x flags %#jx",
+ ldvp, ldvp->v_vflag, dvp, dvp->v_vflag,
+ (uintmax_t)flags));
+ return (ENOENT);
+ }
+ ndp = vfs_lookup_nameidata(cnp);
+ if (ndp != NULL && vfs_lookup_isroot(ndp, ldvp))
+ return (ENOENT);
}
/*
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 7618e2575819..01378b06ba55 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -78,6 +78,21 @@
VNASSERT(((vp)->v_op == &unionfs_vnodeops), vp, \
("%s: non-unionfs vnode", __func__))
+static bool
+unionfs_lookup_isroot(struct componentname *cnp, struct vnode *dvp)
+{
+ struct nameidata *ndp;
+
+ if (dvp == NULL)
+ return (false);
+ if ((dvp->v_vflag & VV_ROOT) != 0)
+ return (true);
+ ndp = vfs_lookup_nameidata(cnp);
+ if (ndp == NULL)
+ return (false);
+ return (vfs_lookup_isroot(ndp, dvp));
+}
+
static int
unionfs_lookup(struct vop_cachedlookup_args *ap)
{
@@ -128,6 +143,12 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
if (LOOKUP != nameiop && udvp == NULLVP)
return (EROFS);
+ if (unionfs_lookup_isroot(cnp, udvp) ||
+ unionfs_lookup_isroot(cnp, ldvp)) {
+ error = ENOENT;
+ goto unionfs_lookup_return;
+ }
+
if (udvp != NULLVP) {
dtmpvp = udvp;
if (ldvp != NULLVP)
diff --git a/sys/kern/vfs_cache.c b/sys/kern/vfs_cache.c
index e25efe2986b2..2c2e18640a76 100644
--- a/sys/kern/vfs_cache.c
+++ b/sys/kern/vfs_cache.c
@@ -5185,7 +5185,6 @@ cache_fplookup_dotdot(struct cache_fpl *fpl)
struct componentname *cnp;
struct namecache *ncp;
struct vnode *dvp;
- struct prison *pr;
u_char nc_flag;
ndp = fpl->ndp;
@@ -5197,15 +5196,7 @@ cache_fplookup_dotdot(struct cache_fpl *fpl)
/*
* XXX this is racy the same way regular lookup is
*/
- for (pr = cnp->cn_cred->cr_prison; pr != NULL;
- pr = pr->pr_parent)
- if (dvp == pr->pr_root)
- break;
-
- if (dvp == ndp->ni_rootdir ||
- dvp == ndp->ni_topdir ||
- dvp == rootvnode ||
- pr != NULL) {
+ if (vfs_lookup_isroot(ndp, dvp)) {
fpl->tvp = dvp;
fpl->tvp_seqc = vn_seqc_read_any(dvp);
if (seqc_in_modify(fpl->tvp_seqc)) {
diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c
index b7f751a364ce..1d303fecf493 100644
--- a/sys/kern/vfs_lookup.c
+++ b/sys/kern/vfs_lookup.c
@@ -612,12 +612,12 @@ namei(struct nameidata *ndp)
}
#endif
ndp->ni_cnd.cn_cred = td->td_ucred;
- KASSERT(ndp->ni_resflags == 0, ("%s: garbage in ni_resflags: %x\n",
+ KASSERT(ndp->ni_resflags == 0, ("%s: garbage in ni_resflags: %x",
__func__, ndp->ni_resflags));
KASSERT(cnp->cn_cred && td->td_proc, ("namei: bad cred/proc"));
KASSERT((cnp->cn_flags & NAMEI_INTERNAL_FLAGS) == 0,
- ("namei: unexpected flags: %" PRIx64 "\n",
- cnp->cn_flags & NAMEI_INTERNAL_FLAGS));
+ ("namei: unexpected flags: %#jx",
+ (uintmax_t)(cnp->cn_flags & NAMEI_INTERNAL_FLAGS)));
if (cnp->cn_flags & NOCACHE)
KASSERT(cnp->cn_nameiop != LOOKUP,
("%s: NOCACHE passed with LOOKUP", __func__));
@@ -863,6 +863,30 @@ bad:
return (error);
}
+struct nameidata *
+vfs_lookup_nameidata(struct componentname *cnp)
+{
+ if ((cnp->cn_flags & NAMEILOOKUP) == 0)
+ return (NULL);
+ return (__containerof(cnp, struct nameidata, ni_cnd));
+}
+
+/*
+ * Would a dotdot lookup relative to dvp cause this lookup to cross a jail or
+ * chroot boundary?
+ */
+bool
+vfs_lookup_isroot(struct nameidata *ndp, struct vnode *dvp)
+{
+ for (struct prison *pr = ndp->ni_cnd.cn_cred->cr_prison; pr != NULL;
+ pr = pr->pr_parent) {
+ if (dvp == pr->pr_root)
+ return (true);
+ }
+ return (dvp == ndp->ni_rootdir || dvp == ndp->ni_topdir ||
+ dvp == rootvnode);
+}
+
/*
* FAILIFEXISTS handling.
*
@@ -1021,7 +1045,6 @@ vfs_lookup(struct nameidata *ndp)
char *lastchar; /* location of the last character */
struct vnode *dp = NULL; /* the directory we are searching */
struct vnode *tdp; /* saved dp */
- struct prison *pr;
size_t prev_ni_pathlen; /* saved ndp->ni_pathlen */
int docache; /* == 0 do not cache last component */
int wantparent; /* 1 => wantparent or lockparent flag */
@@ -1207,13 +1230,9 @@ dirloop:
goto bad;
}
for (;;) {
- for (pr = cnp->cn_cred->cr_prison; pr != NULL;
- pr = pr->pr_parent)
- if (dp == pr->pr_root)
- break;
- bool isroot = dp == ndp->ni_rootdir ||
- dp == ndp->ni_topdir || dp == rootvnode ||
- pr != NULL;
+ bool isroot;
+
+ isroot = vfs_lookup_isroot(ndp, dp);
if (__predict_false(isroot && (ndp->ni_lcf &
(NI_LCF_STRICTREL | NI_LCF_STRICTREL_KTR)) != 0)) {
if ((ndp->ni_lcf & NI_LCF_STRICTREL_KTR) != 0)
diff --git a/sys/sys/namei.h b/sys/sys/namei.h
index 3db7b8e13749..5e9a5bf4a699 100644
--- a/sys/sys/namei.h
+++ b/sys/sys/namei.h
@@ -152,6 +152,7 @@ int cache_fplookup(struct nameidata *ndp, enum
cache_fpl_status *status,
#define LOCKSHARED 0x0100 /* Shared lock leaf */
#define NOFOLLOW 0x0000 /* do not follow symbolic links
(pseudo) */
#define RBENEATH 0x100000000ULL /* No escape, even tmp, from
start dir */
+#define NAMEILOOKUP 0x200000000ULL /* cnp is embedded in nameidata
*/
#define MODMASK 0xf000001ffULL /* mask of operational
modifiers */
/*
@@ -248,7 +249,7 @@ do {
\
NDINIT_PREFILL(_ndp);
\
NDINIT_DBG(_ndp);
\
_ndp->ni_cnd.cn_nameiop = op;
\
- _ndp->ni_cnd.cn_flags = flags;
\
+ _ndp->ni_cnd.cn_flags = (flags) | NAMEILOOKUP;
\
_ndp->ni_segflg = segflg;
\
_ndp->ni_dirp = namep;
\
_ndp->ni_dirfd = dirfd;
\
@@ -285,6 +286,8 @@ do {
\
int namei(struct nameidata *ndp);
int vfs_lookup(struct nameidata *ndp);
+bool vfs_lookup_isroot(struct nameidata *ndp, struct vnode *dvp);
+struct nameidata *vfs_lookup_nameidata(struct componentname *cnp);
int vfs_relookup(struct vnode *dvp, struct vnode **vpp,
struct componentname *cnp, bool refstart);
