Page MenuHomeFreeBSD

vfs: return a global path from vn_path_to_global_path_hardlink()
Needs ReviewPublic

Authored by olgeni on Sun, Sep 6, 9:24 AM.

Details

Reviewers
dfr
kib
mjg
Summary

vfs_domount() records the mount point in f_mntonname, choosing how to
resolve it by vnode type: vn_path_to_global_path() for a directory, and
vn_path_to_global_path_hardlink() for a regular file, which is what a
nullfs file mount lands on.

Unlike its directory counterpart, the hardlink variant does not produce a
global path. It calls vn_fullpath_hardlink(), which resolves against the
calling thread's root directory (pwd->pwd_rdir) rather than against
rootvnode, so a chrooted or jailed caller records a path relative to its
own root while every consumer treats f_mntonname as global.

Inside a jail this makes the mount impossible to remove, two ways over:

  • prison_canseemount() compares f_mntonname against the jail's pr_path. It no longer matches, so the jail cannot see its own mount and prison_enforce_statfs() blanks f_fsid and reports "[restricted]". unmount(2) with MNT_BYFSID is therefore unavailable as well.
  • kern_unmount() globalizes the path it is given and compares it against f_mntonname. That never matches, so unmount(2) by path returns EINVAL. From outside the jail the recorded path does not resolve either.

The mount ends up with no path in any namespace, and only
"umount -f <fsid>" issued from the host can remove it.

A directory mount made the same way records its global path, is visible
from inside the jail, and unmounts normally. That asymmetry is the bug.

Practical impact: this leaks two mounts per container for podman and
buildah on FreeBSD, whose OCI runtime (sysutils/ocijail) nullfs-mounts
/etc/resolv.conf and /run/.containerenv as files. It also breaks
"podman build --squash", which reuses one rootfs directory across build
steps, so the leaked mount from step N sits exactly where step N+1 wants
to mount and the build fails with EBUSY.

The fix gives the shared implementation a flag selecting the root to
resolve against. vn_fullpath_hardlink() keeps its KPI and its behaviour
for the other callers - kern___realpathat(), and AT_EXECPATH in
kern_exec.c and kern_proc.c - which all want the caller's view.

Introduced in fa7217a71232 / 78d35459a258, which added
vn_path_to_global_path_hardlink() for this file-mount case and reused
vn_fullpath_hardlink() without accounting for the different root. The
affected code is identical in main, stable/15, stable/14 and
releng/15.1.

Test Plan

Reproducer, in a jail with allow.mount and allow.mount.nullfs:

:> /tmp/src; :> /tmp/dst
mount -t nullfs /tmp/src /tmp/dst
mount -p | grep /tmp/dst     # before: not listed
umount /tmp/dst              # before: "unknown file system" (EINVAL)

Before, from the host, f_mntonname reads "/tmp/dst"; the raw syscall
unmount("/tmp/dst", MNT_FORCE) returns EINVAL, and the mount leaks. A
directory mount at the same path behaves correctly, which isolates the
file-mount path.

After: f_mntonname reads the host-absolute path, the mount is listed
inside the jail as /tmp/dst, and umount from inside the jail succeeds
with no leak.

Runtime tested on releng/15.1 (15.1-RELEASE-p3, amd64), where this code
is identical to main: GENERIC built with the patch, installed and
booted. On that kernel, three consecutive
"podman run --rm --network=host --no-hosts <image> true" in a bastille
jail, counting leftover mounts under containers/storage:

stock kernel:    9 leaked  (fdescfs + resolv.conf + .containerenv)
patched kernel:  3 leaked  (fdescfs only - a separate ocijail bug)

and "podman build --squash", which failed at the second RUN with
"Device busy" on /etc/resolv.conf, now completes and leaves nothing
behind. The remaining fdescfs leak is an unrelated teardown-ordering bug
in ocijail, patched separately upstream; with both, the count is zero.

Builds: GENERIC/amd64 buildkernel is clean with -Werror on main (this
revision's base, targeting freebsd16.0) and on releng/15.1.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 76590
Build 73473: arc lint + arc unit

Event Timeline

olgeni held this revision as a draft.
olgeni published this revision for review.Sun, Sep 6, 9:51 AM

This popped up while building containers in a jail :)