On Tue, Jun 10, 2025 at 09:21:28AM +0100, Al Viro wrote:
> checks if mount is the root of an anonymouns namespace.
> Switch open-coded equivalents to using it.
>
> For mounts that belong to anon namespace !mnt_has_parent(mount)
> is the same as mount == ns->root, and intent is more obvious in
> the latter form.
>
> NB: comment in do_mount_setattr() appears to be very confused...
The comment just mentions a single case where we did regress userspace
some time ago because we didn't allowing changing mount properties on
the real rootfs (And we have this discussion on another thread.).
But I'm not sure why this belongs in the commit message in the first
place. Just remove the comment.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx>
> fs/mount.h | 7 +++++++
> fs/namespace.c | 17 +++--------------
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/fs/mount.h b/fs/mount.h
> index 9fe06e901cc8..18fa88ad752a 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -160,6 +160,13 @@ static inline bool is_anon_ns(struct mnt_namespace *ns)
> return ns->seq == 0;
> }
>
> +static inline bool anon_ns_root(const struct mount *m)
> +{
> + struct mnt_namespace *ns = READ_ONCE(m->mnt_ns);
> +
> + return !IS_ERR_OR_NULL(ns) && is_anon_ns(ns) && m == ns->root;
> +}
> +
> static inline bool mnt_ns_attached(const struct mount *mnt)
> {
> return !RB_EMPTY_NODE(&mnt->mnt_node);
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 2fb5b9fcd2cd..b229f74762de 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2485,9 +2485,7 @@ struct vfsmount *clone_private_mount(const struct path *path)
> * loops get created.
> */
> if (!check_mnt(old_mnt)) {
> - if (!is_mounted(&old_mnt->mnt) ||
> - !is_anon_ns(old_mnt->mnt_ns) ||
> - mnt_has_parent(old_mnt))
> + if (!anon_ns_root(old_mnt))
> return ERR_PTR(-EINVAL);
>
> if (!check_for_nsfs_mounts(old_mnt))
> @@ -3657,9 +3655,6 @@ static int do_move_mount(struct path *old_path,
> ns = old->mnt_ns;
>
> err = -EINVAL;
> - /* The thing moved must be mounted... */
> - if (!is_mounted(&old->mnt))
> - goto out;
>
> if (check_mnt(old)) {
> /* if the source is in our namespace... */
> @@ -3672,10 +3667,8 @@ static int do_move_mount(struct path *old_path,
> } else {
> /*
> * otherwise the source must be the root of some anon namespace.
> - * AV: check for mount being root of an anon namespace is worth
> - * an inlined predicate...
> */
> - if (!is_anon_ns(ns) || mnt_has_parent(old))
> + if (!anon_ns_root(old))
> goto out;
> /*
> * Bail out early if the target is within the same namespace -
> @@ -5036,10 +5029,6 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
> err = -EINVAL;
> lock_mount_hash();
>
> - /* Ensure that this isn't anything purely vfs internal. */
> - if (!is_mounted(&mnt->mnt))
> - goto out;
> -
> /*
> * If this is an attached mount make sure it's located in the callers
> * mount namespace. If it's not don't let the caller interact with it.
> @@ -5051,7 +5040,7 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
> * neither has a parent nor is it a detached mount so we cannot
> * unconditionally check for detached mounts.
> */
> - if ((mnt_has_parent(mnt) || !is_anon_ns(mnt->mnt_ns)) && !check_mnt(mnt))
> + if (!anon_ns_root(mnt) && !check_mnt(mnt))
> goto out;
>
> /*
> --
> 2.39.5
>
