[Devel] [PATCH vz10 1/3] ve/fs: unlink the mount namespace on the copy_mnt_ns() error path

[Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>]

alloc_mnt_ns() does two things upstream does not: it links the namespace
onto all_mntns_list and takes a reference on its owning VE. Both are
undone in free_mnt_ns(), which holds the only list_del() of mntns_list
and the only put_ve(ns->ve_owner) in the file.

copy_mnt_ns() does not call it when copy_tree() fails. It open codes the
teardown and finishes with mnt_ns_release(), which drops the passive
count and kfree()s the namespace without unlinking it, so the namespace
is freed while all_mntns_list still points at it and the VE reference is
leaked. The next namespace creation runs list_add_tail() through the
dangling entry.

A container reaches this deterministically. alloc_vfsmnt() returns NULL
when !ve_mount_allowed(), that is when the VE is at sysctl_ve_mount_nr,
clone_mnt() turns that into -ENOMEM and copy_tree() propagates it. So a
container sitting at its own mount limit that calls unshare(CLONE_NEWNS)
takes the error path every time, with no memory pressure and nothing
beyond CAP_SYS_ADMIN in its own user namespace, and panics the host:

  list_add corruption. prev->next should be next (ffffffffa9ca77f0), but
  was ff2834a3cdfaeed0. (prev=ff2834a3cdfaeed0).
  kernel BUG at lib/list_debug.c:32!
  CPU: 94 UID: 0 PID: 7139 Comm: unshare ve: 900
   alloc_mnt_ns+0xd5/0x210
   copy_mnt_ns+0x82/0x3c0
   create_new_namespaces+0x5d/0x2f0
   unshare_nsproxy_namespaces+0x69/0xc0
   ksys_unshare+0x213/0x3f0

prev->next == prev is INIT_LIST_HEAD() on reallocated memory, the freed
namespace reused while the list still referenced it. CONFIG_DEBUG_LIST is
only what makes it a clean BUG, without it the same list_add_tail()
writes through the dangling pointer silently.

The path used to call free_mnt_ns() and was correct. Upstream replaced
that with the open coded sequence because free_mnt_ns() reaches
mnt_ns_tree_remove(), which rb_erase()s a node that copy_mnt_ns() has not
inserted yet, mnt_ns_tree_add() running only after the copy succeeds.
Upstream is unaffected by the replacement because its free_mnt_ns()
carries nothing else. Ours does.

So do not restore the free_mnt_ns() call, that would reintroduce the
rb_erase() upstream fixed. Split the part that is ours into
mnt_ns_unlink() and call it from both places, so a future addition to
namespace teardown has one home rather than two that can drift apart,
which is how this happened.

Fixes: 229fd15908fe ("fs: don't try and remove empty rbtree node")
Fixes: 1db60e545f65 ("ve/mntns: add ve_owner to struct mnt_namespace")
https://virtuozzo.atlassian.net/browse/VSTOR-141545
Feature: ve: ve generic structures
Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
 fs/namespace.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4d4dc5290350..7e27537dcdaf 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4243,17 +4243,30 @@ static void dec_mnt_namespaces(struct ucounts *ucounts)
 static LIST_HEAD(all_mntns_list);
 static DEFINE_SPINLOCK(all_mntns_list_lock);
 
-static void free_mnt_ns(struct mnt_namespace *ns)
+/*
+ * Undo the bookkeeping alloc_mnt_ns() sets up beyond what upstream does: the
+ * entry on all_mntns_list and the reference on the owning VE.
+ *
+ * Kept separate from free_mnt_ns() because copy_mnt_ns() has to unwind a
+ * namespace that is not in mnt_ns_tree yet, so it cannot use free_mnt_ns()
+ * without rb_erase()ing a node that was never inserted.
+ */
+static void mnt_ns_unlink(struct mnt_namespace *ns)
 {
-	if (!is_anon_ns(ns))
-		ns_free_inum(&ns->ns);
-	dec_mnt_namespaces(ns->ucounts);
-
 	spin_lock(&all_mntns_list_lock);
 	list_del(&ns->mntns_list);
 	spin_unlock(&all_mntns_list_lock);
 
 	put_ve(ns->ve_owner);
+}
+
+static void free_mnt_ns(struct mnt_namespace *ns)
+{
+	if (!is_anon_ns(ns))
+		ns_free_inum(&ns->ns);
+	dec_mnt_namespaces(ns->ucounts);
+
+	mnt_ns_unlink(ns);
 
 	mnt_ns_tree_remove(ns);
 }
@@ -4347,6 +4360,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 		namespace_unlock();
 		ns_free_inum(&new_ns->ns);
 		dec_mnt_namespaces(new_ns->ucounts);
+		mnt_ns_unlink(new_ns);
 		mnt_ns_release(new_ns);
 		return ERR_CAST(new);
 	}
-- 
2.43.0