Re: [PATCH v5] statmount: accept fd as a parameter

On Sun, 9 Nov 2025 at 06:40, Bhavik Sachdev <[email protected]> wrote:

> diff --git a/fs/namespace.c b/fs/namespace.c
> index d82910f33dc4..153c0ea85386 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -5207,6 +5207,12 @@ static int statmount_mnt_root(struct kstatmount *s, struct seq_file *seq)
>         return 0;
>  }
>
> +static int statmount_mnt_point_unmounted(struct kstatmount *s, struct seq_file *seq)
> +{
> +       seq_puts(seq, "[unmounted]");

Please no, There's statmount.mask for this purpose exactly.

>  static int statmount_mnt_point(struct kstatmount *s, struct seq_file *seq)
>  {
>         struct vfsmount *mnt = s->mnt;
> @@ -5262,7 +5268,10 @@ static int statmount_sb_source(struct kstatmount *s, struct seq_file *seq)
>  static void statmount_mnt_ns_id(struct kstatmount *s, struct mnt_namespace *ns)
>  {
>         s->sm.mask |= STATMOUNT_MNT_NS_ID;
> -       s->sm.mnt_ns_id = ns->ns.ns_id;
> +       if (ns)
> +               s->sm.mnt_ns_id = ns->ns.ns_id;
> +       else
> +               s->sm.mnt_ns_id = 0;

Same here.  Please clear mask if the requested field cannot be meaningfully set.

>  /* locks: namespace_shared */
>  static int do_statmount(struct kstatmount *s, u64 mnt_id, u64 mnt_ns_id,
> -                       struct mnt_namespace *ns)
> +                       struct mnt_namespace *ns, unsigned int flags)
>  {
>         struct mount *m;
>         int err;
>
>         /* Has the namespace already been emptied? */
> -       if (mnt_ns_id && mnt_ns_empty(ns))
> +       if (!(flags & STATMOUNT_BY_FD) && mnt_ns_id && mnt_ns_empty(ns))
>                 return -ENOENT;

This check does not make sense without the lookup_mnt_in_ns(), so
please move it right before the lookup.

>
> -       s->mnt = lookup_mnt_in_ns(mnt_id, ns);
> -       if (!s->mnt)
> -               return -ENOENT;
> +       if (!(flags & STATMOUNT_BY_FD)) {
> +               s->mnt = lookup_mnt_in_ns(mnt_id, ns);

A comment would be nice about s->mnt being set in caller for the
STATMOUNT_BY_FD case.

> -       ns = grab_requested_mnt_ns(&kreq);
> -       if (!ns)
> -               return -ENOENT;
> +       if (flags & STATMOUNT_BY_FD) {
> +               file_from_fd = fget_raw(kreq.fd);
> +               if (!file_from_fd)
> +                       return -EBADF;
> +
> +               fd_mnt = file_from_fd->f_path.mnt;
> +               ns = real_mount(fd_mnt)->mnt_ns;
> +               if (ns)
> +                       refcount_inc(&ns->passive);
> +               else
> +                       if (!capable(CAP_SYS_ADMIN))
> +                               return -ENOENT;

Why is this not EPERM?

I think this check belongs together with the  ns_capable_noaudit() check below.

> +       } else {
> +               ns = grab_requested_mnt_ns(&kreq, flags);
> +               if (!ns)
> +                       return -ENOENT;
> +       }
>
> -       if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
> +       if (ns && (ns != current->nsproxy->mnt_ns) &&
>             !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
>                 return -ENOENT;

(This should also be EPERM?  Searate patch, though).

Something like this:

        if (ns) {
                if ((ns != current->nsproxy->mnt_ns) &&
                    !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
                        return -ENOENT;
        } else if (!capable(CAP_SYS_ADMIN))
                return -EPERM;
        }

Thanks,
Miklos