On Wed, 2025-06-04 at 00:16 +0100, Al Viro wrote:
> may_decode_fh() is calling has_locked_children() while holding no locks.
> That's an oopsable race...
>
> The rest of the callers are safe since they are holding namespace_sem and
> are guaranteed a positive refcount on the mount in question.
>
> Rename the current has_locked_children() to __has_locked_children(), make
> it static and switch the fs/namespace.c users to it.
>
> Make has_locked_children() a wrapper for __has_locked_children(), calling
> the latter under read_seqlock_excl(&mount_lock).
>
> Fixes: 620c266f3949 ("fhandle: relax open_by_handle_at() permission checks")
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> fs/namespace.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 7c0ebc4f4ef2..a33553bc12d0 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2425,7 +2425,7 @@ void drop_collected_mounts(struct vfsmount *mnt)
> namespace_unlock();
> }
>
> -bool has_locked_children(struct mount *mnt, struct dentry *dentry)
> +static bool __has_locked_children(struct mount *mnt, struct dentry *dentry)
> {
> struct mount *child;
>
> @@ -2439,6 +2439,16 @@ bool has_locked_children(struct mount *mnt, struct dentry *dentry)
> return false;
> }
>
> +bool has_locked_children(struct mount *mnt, struct dentry *dentry)
> +{
> + bool res;
> +
> + read_seqlock_excl(&mount_lock);
> + res = __has_locked_children(mnt, dentry);
> + read_sequnlock_excl(&mount_lock);
> + return res;
> +}
> +
> /*
> * Check that there aren't references to earlier/same mount namespaces in the
> * specified subtree. Such references can act as pins for mount namespaces
> @@ -2499,7 +2509,7 @@ struct vfsmount *clone_private_mount(const struct path *path)
> return ERR_PTR(-EINVAL);
> }
>
> - if (has_locked_children(old_mnt, path->dentry))
> + if (__has_locked_children(old_mnt, path->dentry))
> return ERR_PTR(-EINVAL);
>
> new_mnt = clone_mnt(old_mnt, path->dentry, CL_PRIVATE);
> @@ -3036,7 +3046,7 @@ static struct mount *__do_loopback(struct path *old_path, int recurse)
> if (!may_copy_tree(old_path))
> return mnt;
>
> - if (!recurse && has_locked_children(old, old_path->dentry))
> + if (!recurse && __has_locked_children(old, old_path->dentry))
> return mnt;
>
> if (recurse)
> @@ -3429,7 +3439,7 @@ static int do_set_group(struct path *from_path, struct path *to_path)
> goto out;
>
> /* From mount should not have locked children in place of To's root */
> - if (has_locked_children(from, to->mnt.mnt_root))
> + if (__has_locked_children(from, to->mnt.mnt_root))
> goto out;
>
> /* Setting sharing groups is only allowed on private mounts */
Good catch!
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
