Re: [Devel] [PATCH RHEL10 COMMIT] ve/fs: unlink the mount namespace on the copy_mnt_ns() error path

[Konstantin Khorenko <khorenko@virtuozzo.com>]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.10.vz10
------>
commit 5e3e3c3facbcd29dfa49f7f6ab5aabb39a60d1f9
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Mon Aug 17 11:16:39 2026 +0400

    ve/fs: unlink the mount namespace on the copy_mnt_ns() error path
    
    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>
    Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/namespace.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4d4dc52903508..7e27537dcdaf9 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);
 	}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel