From: Al Viro viro@zeniv.linux.org.uk
[ Upstream commit d29da1a8f119130e6fc7d5d71029d402dabe2cb0 ]
We want to mount beneath the given location. For that operation to make sense, location must be the root of some mount that has something under it. Currently we let it proceed if those requirements are not met, with rather meaningless results, and have that bogosity caught further down the road; let's fail early instead - do_lock_mount() doesn't make sense unless those conditions hold, and checking them there makes things simpler.
Reviewed-by: Christian Brauner brauner@kernel.org Signed-off-by: Al Viro viro@zeniv.linux.org.uk Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - Prevents nonsensical MOVE_MOUNT_BENEATH operations from proceeding when the destination is not a mount root or when the topmost mount has no parent (i.e., namespace root). Previously this could “proceed with meaningless results” and fail later; now it fails early with -EINVAL as intended.
- Key changes - Early validation in do_lock_mount(): - Adds `if (unlikely(beneath) && !path_mounted(path)) return -EINVAL;` so non-mount-root targets are rejected immediately (fs/namespace.c:do_lock_mount()). - Adds a parent check under `mount_lock` in the ‘beneath’ path: `if (unlikely(!mnt_has_parent(m))) { ... return -EINVAL; }` to reject attempts beneath a namespace root before proceeding (fs/namespace.c:do_lock_mount()). - De-duplication: Removes the equivalent checks from can_move_mount_beneath(), centralizing them where the mountpoint and parent are actually determined (fs/namespace.c:can_move_mount_beneath()).
- Context in current tree - The tree already performs an early `beneath && !path_mounted(path)` rejection in do_lock_mount (see `fs/namespace.c:2732`), so moving/keeping this check in do_lock_mount is aligned with the patch’s intent. - The explicit `mnt_has_parent()` guard is not currently enforced at lock acquisition time in do_lock_mount; adding it there (while holding `mount_lock`) closes a race and ensures the operation only proceeds when a real parent exists. - can_move_mount_beneath in this tree already focuses on propagation/relationship checks and does not contain those path/parent assertions (see around `fs/namespace.c:3417`), so consolidating sanity checks into do_lock_mount is consistent and low risk.
- Why it’s a good stable candidate - Bug fix: Enforces semantic preconditions for MOVE_MOUNT_BENEATH, avoiding misleading or late failures. - Small and contained: Changes are limited to fs/namespace.c, mostly simple condition checks and code movement. - No feature or architectural change: Just earlier, clearer validation; the end result remains a failure for invalid usage. - Concurrency-safe: Parent check is done while holding `mount_lock`, reducing race windows between `mount_lock` and `namespace_sem`.
- Regression risk - Low. Users attempting invalid MOVE_MOUNT_BENEATH operations will now get -EINVAL earlier rather than later. Valid usages are unaffected.
fs/namespace.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c index c8c2376bb2424..fa7c034ac4a69 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2785,12 +2785,19 @@ static int do_lock_mount(struct path *path, struct pinned_mountpoint *pinned, bo struct path under = {}; int err = -ENOENT;
+ if (unlikely(beneath) && !path_mounted(path)) + return -EINVAL; + for (;;) { struct mount *m = real_mount(mnt);
if (beneath) { path_put(&under); read_seqlock_excl(&mount_lock); + if (unlikely(!mnt_has_parent(m))) { + read_sequnlock_excl(&mount_lock); + return -EINVAL; + } under.mnt = mntget(&m->mnt_parent->mnt); under.dentry = dget(m->mnt_mountpoint); read_sequnlock_excl(&mount_lock); @@ -3462,8 +3469,6 @@ static bool mount_is_ancestor(const struct mount *p1, const struct mount *p2) * @to: mount under which to mount * @mp: mountpoint of @to * - * - Make sure that @to->dentry is actually the root of a mount under - * which we can mount another mount. * - Make sure that nothing can be mounted beneath the caller's current * root or the rootfs of the namespace. * - Make sure that the caller can unmount the topmost mount ensuring @@ -3485,12 +3490,6 @@ static int can_move_mount_beneath(const struct path *from, *mnt_to = real_mount(to->mnt), *parent_mnt_to = mnt_to->mnt_parent;
- if (!mnt_has_parent(mnt_to)) - return -EINVAL; - - if (!path_mounted(to)) - return -EINVAL; - if (IS_MNT_LOCKED(mnt_to)) return -EINVAL;