Page MenuHomeFreeBSD

namei: Make stackable filesystems check harder for jail roots
ClosedPublic

Authored by markj on May 19 2025, 3:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 30, 10:45 AM
Unknown Object (File)
Wed, Oct 29, 12:46 PM
Unknown Object (File)
Wed, Oct 29, 9:47 AM
Unknown Object (File)
Wed, Oct 29, 9:45 AM
Unknown Object (File)
Sun, Oct 26, 7:48 AM
Unknown Object (File)
Sat, Oct 25, 2:36 PM
Unknown Object (File)
Tue, Oct 14, 3:46 AM
Unknown Object (File)
Sep 28 2025, 2:53 AM
Subscribers

Details

Summary

Suppose a process has its cwd pointing to a nullfs directory. 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 trick 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

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.May 19 2025, 3:59 PM

I appreciate the single vfs_lookup_isroot function - it took me an embarrassingly long time to discover a second copy of that code had appeared in vfs_cache.c a mere five years ago.

A question for the uninitiated about the NAMEILOOKUP flag: what other context are names looked up in, that don't have nameidata?

I appreciate the single vfs_lookup_isroot function - it took me an embarrassingly long time to discover a second copy of that code had appeared in vfs_cache.c a mere five years ago.

A question for the uninitiated about the NAMEILOOKUP flag: what other context are names looked up in, that don't have nameidata?

There are some VOP_LOOKUP calls in individual filesystems which manually fill out a componentname: zfs_fhtovp() contains a couple of examples. I'm not yet completely convinced that it's safe to elide all of those cases, but I'm pretty sure it is.

The check is present for nullfs but not unionfs here. Could you rather wrap the __containerof(cnp, struct nameidata, ni_cnd) into a function that will check for NAMEILOOKUP positioned, and will return NULL if it is not?

sys/fs/nullfs/null_vnops.c
404–412

This comment really applies only to the VV_ROOT case, so I'd move it closer to the if ((ldvp->v_vflag & VV_ROOT) != 0) line.

kib added inline comments.
sys/kern/vfs_lookup.c
868

Please add {} around the body.

This revision is now accepted and ready to land.May 19 2025, 10:34 PM
sys/sys/namei.h
155

Please keep namei.9 up to date.

olce requested changes to this revision.May 20 2025, 10:02 AM

Just to be sure that my comment is not lost:

The check is present for nullfs but not unionfs here. Could you rather wrap the __containerof(cnp, struct nameidata, ni_cnd) into a function that will check for NAMEILOOKUP positioned, and will return NULL if it is not?

The change is not conceptually correct for unionfs.

This revision now requires changes to proceed.May 20 2025, 10:02 AM
markj added inline comments.
sys/fs/nullfs/null_vnops.c
404–412

Most of the comment applies to both cases, so I just tweaked it slightly instead.

markj marked an inline comment as done.

Handle review comments:

  • update namei.8
  • use a helper function to extract nameidata
This revision is now accepted and ready to land.May 22 2025, 3:56 PM