Skip to content

Commit 7587f6d

Browse files
markjdbmarkjdb
committed
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
1 parent fb1749c commit 7587f6d

File tree

6 files changed

+83
-32
lines changed

6 files changed

+83
-32
lines changed

‎share/man/man9/namei.9‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,15 @@ The flag is used to implement
281281
.Dv O_RESOLVE_BENEATH
282282
flag for
283283
.Xr openat 2 .
284+
.It Dv NAMEILOOKUP
285+
The component is embedded in a
286+
.Nm
287+
lookup structure, and the
288+
.Fn vfs_lookup_nameidata
289+
function can be used to obtain that structure.
290+
This can be useful in
291+
.Xr VOP_LOOKUP 9
292+
implementations which need to obtain extra lookup metadata.
284293
.El
285294
.Sh PARAMETERS DESCRIPTORS FLAGS
286295
These flags are used for several purposes.

‎sys/fs/nullfs/null_vnops.c‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -403,17 +403,25 @@ null_lookup(struct vop_lookup_args *ap)
403403

404404
/*
405405
* Renames in the lower mounts might create an inconsistent
406-
* configuration where lower vnode is moved out of the
407-
* directory tree remounted by our null mount. Do not try to
408-
* handle it fancy, just avoid VOP_LOOKUP() with DOTDOT name
409-
* which cannot be handled by VOP, at least passing over lower
410-
* root.
406+
* configuration where lower vnode is moved out of the directory tree
407+
* remounted by our null mount.
408+
*
409+
* Do not try to handle it fancy, just avoid VOP_LOOKUP() with DOTDOT
410+
* name which cannot be handled by the VOP.
411411
*/
412-
if ((ldvp->v_vflag & VV_ROOT) != 0 && (flags & ISDOTDOT) != 0) {
413-
KASSERT((dvp->v_vflag & VV_ROOT) == 0,
414-
("ldvp %p fl %#x dvp %p fl %#x flags %#jx",
415-
ldvp, ldvp->v_vflag, dvp, dvp->v_vflag, (uintmax_t)flags));
416-
return (ENOENT);
412+
if ((flags & ISDOTDOT) != 0) {
413+
struct nameidata *ndp;
414+
415+
if ((ldvp->v_vflag & VV_ROOT) != 0) {
416+
KASSERT((dvp->v_vflag & VV_ROOT) == 0,
417+
("ldvp %p fl %#x dvp %p fl %#x flags %#jx",
418+
ldvp, ldvp->v_vflag, dvp, dvp->v_vflag,
419+
(uintmax_t)flags));
420+
return (ENOENT);
421+
}
422+
ndp = vfs_lookup_nameidata(cnp);
423+
if (ndp != NULL && vfs_lookup_isroot(ndp, ldvp))
424+
return (ENOENT);
417425
}
418426

419427
/*

‎sys/fs/unionfs/union_vnops.c‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,21 @@
7676
VNASSERT(((vp)->v_op == &unionfs_vnodeops), vp, \
7777
("%s: non-unionfs vnode", __func__))
7878

79+
static bool
80+
unionfs_lookup_isroot(struct componentname *cnp, struct vnode *dvp)
81+
{
82+
struct nameidata *ndp;
83+
84+
if (dvp == NULL)
85+
return (false);
86+
if ((dvp->v_vflag & VV_ROOT) != 0)
87+
return (true);
88+
ndp = vfs_lookup_nameidata(cnp);
89+
if (ndp == NULL)
90+
return (false);
91+
return (vfs_lookup_isroot(ndp, dvp));
92+
}
93+
7994
static int
8095
unionfs_lookup(struct vop_cachedlookup_args *ap)
8196
{
@@ -149,6 +164,12 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
149164
goto unionfs_lookup_return;
150165
}
151166

167+
if (unionfs_lookup_isroot(cnp, udvp) ||
168+
unionfs_lookup_isroot(cnp, ldvp)) {
169+
error = ENOENT;
170+
goto unionfs_lookup_return;
171+
}
172+
152173
if (udvp != NULLVP)
153174
dtmpvp = udvp;
154175
else

‎sys/kern/vfs_cache.c‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5276,7 +5276,6 @@ cache_fplookup_dotdot(struct cache_fpl *fpl)
52765276
struct componentname *cnp;
52775277
struct namecache *ncp;
52785278
struct vnode *dvp;
5279-
struct prison *pr;
52805279
u_char nc_flag;
52815280

52825281
ndp = fpl->ndp;
@@ -5288,15 +5287,7 @@ cache_fplookup_dotdot(struct cache_fpl *fpl)
52885287
/*
52895288
* XXX this is racy the same way regular lookup is
52905289
*/
5291-
for (pr = cnp->cn_cred->cr_prison; pr != NULL;
5292-
pr = pr->pr_parent)
5293-
if (dvp == pr->pr_root)
5294-
break;
5295-
5296-
if (dvp == ndp->ni_rootdir ||
5297-
dvp == ndp->ni_topdir ||
5298-
dvp == rootvnode ||
5299-
pr != NULL) {
5290+
if (vfs_lookup_isroot(ndp, dvp)) {
53005291
fpl->tvp = dvp;
53015292
fpl->tvp_seqc = vn_seqc_read_any(dvp);
53025293
if (seqc_in_modify(fpl->tvp_seqc)) {

‎sys/kern/vfs_lookup.c‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -611,12 +611,12 @@ namei(struct nameidata *ndp)
611611
}
612612
#endif
613613
ndp->ni_cnd.cn_cred = td->td_ucred;
614-
KASSERT(ndp->ni_resflags == 0, ("%s: garbage in ni_resflags: %x\n",
614+
KASSERT(ndp->ni_resflags == 0, ("%s: garbage in ni_resflags: %x",
615615
__func__, ndp->ni_resflags));
616616
KASSERT(cnp->cn_cred && td->td_proc, ("namei: bad cred/proc"));
617617
KASSERT((cnp->cn_flags & NAMEI_INTERNAL_FLAGS) == 0,
618-
("namei: unexpected flags: %" PRIx64 "\n",
619-
cnp->cn_flags & NAMEI_INTERNAL_FLAGS));
618+
("namei: unexpected flags: %#jx",
619+
(uintmax_t)(cnp->cn_flags & NAMEI_INTERNAL_FLAGS)));
620620
if (cnp->cn_flags & NOCACHE)
621621
KASSERT(cnp->cn_nameiop != LOOKUP,
622622
("%s: NOCACHE passed with LOOKUP", __func__));
@@ -862,6 +862,30 @@ vfs_lookup_degenerate(struct nameidata *ndp, struct vnode *dp, int wantparent)
862862
return (error);
863863
}
864864

865+
struct nameidata *
866+
vfs_lookup_nameidata(struct componentname *cnp)
867+
{
868+
if ((cnp->cn_flags & NAMEILOOKUP) == 0)
869+
return (NULL);
870+
return (__containerof(cnp, struct nameidata, ni_cnd));
871+
}
872+
873+
/*
874+
* Would a dotdot lookup relative to dvp cause this lookup to cross a jail or
875+
* chroot boundary?
876+
*/
877+
bool
878+
vfs_lookup_isroot(struct nameidata *ndp, struct vnode *dvp)
879+
{
880+
for (struct prison *pr = ndp->ni_cnd.cn_cred->cr_prison; pr != NULL;
881+
pr = pr->pr_parent) {
882+
if (dvp == pr->pr_root)
883+
return (true);
884+
}
885+
return (dvp == ndp->ni_rootdir || dvp == ndp->ni_topdir ||
886+
dvp == rootvnode);
887+
}
888+
865889
/*
866890
* FAILIFEXISTS handling.
867891
*
@@ -1020,7 +1044,6 @@ vfs_lookup(struct nameidata *ndp)
10201044
char *lastchar; /* location of the last character */
10211045
struct vnode *dp = NULL; /* the directory we are searching */
10221046
struct vnode *tdp; /* saved dp */
1023-
struct prison *pr;
10241047
size_t prev_ni_pathlen; /* saved ndp->ni_pathlen */
10251048
int docache; /* == 0 do not cache last component */
10261049
int wantparent; /* 1 => wantparent or lockparent flag */
@@ -1206,13 +1229,9 @@ vfs_lookup(struct nameidata *ndp)
12061229
goto bad;
12071230
}
12081231
for (;;) {
1209-
for (pr = cnp->cn_cred->cr_prison; pr != NULL;
1210-
pr = pr->pr_parent)
1211-
if (dp == pr->pr_root)
1212-
break;
1213-
bool isroot = dp == ndp->ni_rootdir ||
1214-
dp == ndp->ni_topdir || dp == rootvnode ||
1215-
pr != NULL;
1232+
bool isroot;
1233+
1234+
isroot = vfs_lookup_isroot(ndp, dp);
12161235
if (__predict_false(isroot && (ndp->ni_lcf &
12171236
(NI_LCF_STRICTREL | NI_LCF_STRICTREL_KTR)) != 0)) {
12181237
if ((ndp->ni_lcf & NI_LCF_STRICTREL_KTR) != 0)

‎sys/sys/namei.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ int cache_fplookup(struct nameidata *ndp, enum cache_fpl_status *status,
152152
#define LOCKSHARED 0x0100 /* Shared lock leaf */
153153
#define NOFOLLOW 0x0000 /* do not follow symbolic links (pseudo) */
154154
#define RBENEATH 0x100000000ULL /* No escape, even tmp, from start dir */
155+
#define NAMEILOOKUP 0x200000000ULL /* cnp is embedded in nameidata */
155156
#define MODMASK 0xf000001ffULL /* mask of operational modifiers */
156157

157158
/*
@@ -249,7 +250,7 @@ do { \
249250
NDINIT_PREFILL(_ndp); \
250251
NDINIT_DBG(_ndp); \
251252
_ndp->ni_cnd.cn_nameiop = op; \
252-
_ndp->ni_cnd.cn_flags = flags; \
253+
_ndp->ni_cnd.cn_flags = (flags) | NAMEILOOKUP; \
253254
_ndp->ni_segflg = segflg; \
254255
_ndp->ni_dirp = namep; \
255256
_ndp->ni_dirfd = dirfd; \
@@ -286,6 +287,8 @@ do { \
286287

287288
int namei(struct nameidata *ndp);
288289
int vfs_lookup(struct nameidata *ndp);
290+
bool vfs_lookup_isroot(struct nameidata *ndp, struct vnode *dvp);
291+
struct nameidata *vfs_lookup_nameidata(struct componentname *cnp);
289292
int vfs_relookup(struct vnode *dvp, struct vnode **vpp,
290293
struct componentname *cnp, bool refstart);
291294

0 commit comments

Comments
 (0)