listmount: don't call path_put() under namespace semaphore

Massage listmount() and make sure we don't call path_put() under the
namespace semaphore. If we put the last reference we're fscked.

Fixes: b4c2bea8ceaa ("add listmount(2) syscall")
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/namespace.c b/fs/namespace.c
index 993983a768536110e4cd133ae9c4812522cfb0ed..6686c9f54b40059434cf5543287435f6d9fb48d7 100644 (file)
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5963,23 +5963,34 @@ retry:
        return ret;
 }
 
-static ssize_t do_listmount(struct mnt_namespace *ns, u64 mnt_parent_id,
-                           u64 last_mnt_id, u64 *mnt_ids, size_t nr_mnt_ids,
-                           bool reverse)
+struct klistmount {
+       u64 last_mnt_id;
+       u64 mnt_parent_id;
+       u64 *kmnt_ids;
+       u32 nr_mnt_ids;
+       struct mnt_namespace *ns;
+       struct path root;
+};
+
+static ssize_t do_listmount(struct klistmount *kls, bool reverse)
 {
-       struct path root __free(path_put) = {};
+       struct mnt_namespace *ns = kls->ns;
+       u64 mnt_parent_id = kls->mnt_parent_id;
+       u64 last_mnt_id = kls->last_mnt_id;
+       u64 *mnt_ids = kls->kmnt_ids;
+       size_t nr_mnt_ids = kls->nr_mnt_ids;
        struct path orig;
        struct mount *r, *first;
        ssize_t ret;
 
        rwsem_assert_held(&namespace_sem);
 
-       ret = grab_requested_root(ns, &root);
+       ret = grab_requested_root(ns, &kls->root);
        if (ret)
                return ret;
 
        if (mnt_parent_id == LSMT_ROOT) {
-               orig = root;
+               orig = kls->root;
        } else {
                orig.mnt = lookup_mnt_in_ns(mnt_parent_id, ns);
                if (!orig.mnt)
@@ -5991,7 +6002,7 @@ static ssize_t do_listmount(struct mnt_namespace *ns, u64 mnt_parent_id,
         * Don't trigger audit denials. We just want to determine what
         * mounts to show users.
         */
-       if (!is_path_reachable(real_mount(orig.mnt), orig.dentry, &root) &&
+       if (!is_path_reachable(real_mount(orig.mnt), orig.dentry, &kls->root) &&
            !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
                return -EPERM;
 
@@ -6024,14 +6035,45 @@ static ssize_t do_listmount(struct mnt_namespace *ns, u64 mnt_parent_id,
        return ret;
 }
 
+static void __free_klistmount_free(const struct klistmount *kls)
+{
+       path_put(&kls->root);
+       kvfree(kls->kmnt_ids);
+       mnt_ns_release(kls->ns);
+}
+
+static inline int prepare_klistmount(struct klistmount *kls, struct mnt_id_req *kreq,
+                                    size_t nr_mnt_ids)
+{
+
+       u64 last_mnt_id = kreq->param;
+
+       /* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
+       if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
+               return -EINVAL;
+
+       kls->last_mnt_id = last_mnt_id;
+
+       kls->nr_mnt_ids = nr_mnt_ids;
+       kls->kmnt_ids = kvmalloc_array(nr_mnt_ids, sizeof(*kls->kmnt_ids),
+                                      GFP_KERNEL_ACCOUNT);
+       if (!kls->kmnt_ids)
+               return -ENOMEM;
+
+       kls->ns = grab_requested_mnt_ns(kreq);
+       if (!kls->ns)
+               return -ENOENT;
+
+       kls->mnt_parent_id = kreq->mnt_id;
+       return 0;
+}
+
 SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
                u64 __user *, mnt_ids, size_t, nr_mnt_ids, unsigned int, flags)
 {
-       u64 *kmnt_ids __free(kvfree) = NULL;
+       struct klistmount kls __free(klistmount_free) = {};
        const size_t maxcount = 1000000;
-       struct mnt_namespace *ns __free(mnt_ns_release) = NULL;
        struct mnt_id_req kreq;
-       u64 last_mnt_id;
        ssize_t ret;
 
        if (flags & ~LISTMOUNT_REVERSE)
@@ -6052,22 +6094,12 @@ SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
        if (ret)
                return ret;
 
-       last_mnt_id = kreq.param;
-       /* The first valid unique mount id is MNT_UNIQUE_ID_OFFSET + 1. */
-       if (last_mnt_id != 0 && last_mnt_id <= MNT_UNIQUE_ID_OFFSET)
-               return -EINVAL;
-
-       kmnt_ids = kvmalloc_array(nr_mnt_ids, sizeof(*kmnt_ids),
-                                 GFP_KERNEL_ACCOUNT);
-       if (!kmnt_ids)
-               return -ENOMEM;
-
-       ns = grab_requested_mnt_ns(&kreq);
-       if (!ns)
-               return -ENOENT;
+       ret = prepare_klistmount(&kls, &kreq, nr_mnt_ids);
+       if (ret)
+               return ret;
 
-       if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
-           !ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
+       if (kreq.mnt_ns_id && (kls.ns != current->nsproxy->mnt_ns) &&
+           !ns_capable_noaudit(kls.ns->user_ns, CAP_SYS_ADMIN))
                return -ENOENT;
 
        /*
@@ -6075,12 +6107,11 @@ SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
         * listmount() doesn't care about any mount properties.
         */
        scoped_guard(rwsem_read, &namespace_sem)
-               ret = do_listmount(ns, kreq.mnt_id, last_mnt_id, kmnt_ids,
-                                  nr_mnt_ids, (flags & LISTMOUNT_REVERSE));
+               ret = do_listmount(&kls, (flags & LISTMOUNT_REVERSE));
        if (ret <= 0)
                return ret;
 
-       if (copy_to_user(mnt_ids, kmnt_ids, ret * sizeof(*mnt_ids)))
+       if (copy_to_user(mnt_ids, kls.kmnt_ids, ret * sizeof(*mnt_ids)))
                return -EFAULT;
 
        return ret;