Page MenuHomeFreeBSD

Add a generic mechanism for preventing forced unmount
ClosedPublic

Authored by jah on May 23 2021, 3:32 AM.

Details

Summary

This is aimed at preventing stacked filesystems like nullfs and unionfs
from "losing" their lower mounts due to forced unmount. Otherwise,
VFS operations that are passed through to the lower filesystem(s) may
crash or otherwise cause unpredictable behavior.

Introduce two new functions: vfs_pin_from_vp() and vfs_unpin().
which are intended to be called on the lower mount(s) when the stacked
filesystem is mounted and unmounted, respectively.
Much as registration in the mnt_uppers list previously did, pinning
will prevent even forced unmount of the lower FS and will allow the
stacked FS to freely operate on the lower mount either by direct
use of the struct mount* or indirect use through a properly-referenced
vnode's v_mount field.

vfs_pin_from_vp() is modeled after vfs_ref_from_vp() in that it uses
the mount interlock coupled with re-checking vp->v_mount to ensure
that it will fail in the face of a pending unmount request, even if
the concurrent unmount fully completes.

Adopt these new functions in both nullfs and unionfs.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jah requested review of this revision.May 23 2021, 3:32 AM

Should pin imply normal reference on the mp? I suspect that it would simplify reasoning about the mp state.

Should we add some asserts, like MNTK_UNMOUNT implies mnt_pinned_count == 0? E.g., vfs_busy()/vfs_unbusy() might be a good place for the check.

sys/kern/vfs_mount.c
516

Extra () in the expression before ||.

520

It might be worth to add the check that we do not overflow there.

528

We usually do not assert that pointers are not NULL, leaving it to hw to do the check.

531

It makes more sense to assert that count >= 1 before decrement, IMO.

Add and tweak various asserts

This revision is now accepted and ready to land.May 25 2021, 7:23 AM
markj added inline comments.
sys/fs/nullfs/null_vfsops.c
175

Stray newline?

This revision now requires review to proceed.May 25 2021, 5:39 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 6 2021, 1:18 AM
This revision was automatically updated to reflect the committed changes.