[Devel] [PATCH RHEL9 COMMIT] ve/userns: remove all hashed entries before freeing user_namespace

Konstantin Khorenko khorenko at virtuozzo.com
Thu Nov 2 20:36:00 MSK 2023


The commit is pushed to "branch-rh9-5.14.0-284.25.1.vz9.30.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh9-5.14.0-284.25.1.vz9.30.9
------>
commit 70a0c21759a4f1f73fb56797e0198d1356281050
Author: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
Date:   Fri Sep 29 18:48:12 2023 +0300

    ve/userns: remove all hashed entries before freeing user_namespace
    
    548df8b4b57b (ve/userns: associate user_struct with the user_namespace, 2017-03-13)
    introduced dynamically allocated per-userns uid hastable
    instead of using a global static hash table.
    
    The problem with that allocate hashtable is that life cycle of
    the two objects is different - both structes use reference
    counts but they are counted separately. The contained objects
    (user_struct) are not referenced from user_namespace.
    The result of that is that on ve destroy last reference to
    user_namespace is dropped and it is freed - the allocated
    hash table is freed too.
    
    But user_structs that were hashed can still be used elsewhere
    (like on inodes), so when the reference to that structs is dropped
    (evict_inodes) they try to remove from the hash which was alrady freed.
    This results in writting a NULL to an already freed memory.
    But since this happens after some time that write usually corrupts
    some new allocation which results in a variety of crashes.
    
    To fix this remove all user_structs that are still in the hash
    when destroing user_namespace. The hash is used to lookup them
    but since the user_namespace is gone no further lookups are possible.
    
    https://jira.vzint.dev/browse/PSBM-150648
    Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
    
    Feature: userns: make user related resources per userns
---
 include/linux/sched/user.h |  1 +
 kernel/user.c              | 13 +++++++++++++
 kernel/user_namespace.c    |  3 +++
 3 files changed, 17 insertions(+)

diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h
index f4c5bc54d62b..0f45e50f71a2 100644
--- a/include/linux/sched/user.h
+++ b/include/linux/sched/user.h
@@ -43,6 +43,7 @@ struct user_struct {
 extern int uids_sysfs_init(void);
 
 extern struct user_struct *find_user(kuid_t);
+extern void uid_hash_remove_all(struct hlist_head *hlist);
 
 extern struct user_struct root_user;
 #define INIT_USER (&root_user)
diff --git a/kernel/user.c b/kernel/user.c
index ca0f7c78a045..ea2b25293614 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -113,6 +113,19 @@ static void uid_hash_remove(struct user_struct *up)
 	hlist_del_init(&up->uidhash_node);
 }
 
+void uid_hash_remove_all(struct hlist_head *hlist)
+{
+	struct user_struct *up;
+	const struct hlist_node *tmp;
+	unsigned long flags;
+
+	spin_lock_irqsave(&uidhash_lock, flags);
+	if (!hlist_empty(hlist))
+		hlist_for_each_entry_safe(up, tmp, hlist, uidhash_node)
+			hlist_del_init(&up->uidhash_node);
+	spin_unlock_irqrestore(&uidhash_lock, flags);
+}
+
 static struct user_struct *uid_hash_find(kuid_t uid, struct hlist_head *hashent)
 {
 	struct user_struct *user;
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index e175452253d3..52e6bff36abf 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -205,6 +205,7 @@ static void free_user_ns(struct work_struct *work)
 {
 	struct user_namespace *parent, *ns =
 		container_of(work, struct user_namespace, work);
+	int i;
 
 	do {
 		struct ucounts *ucounts = ns->ucounts;
@@ -224,6 +225,8 @@ static void free_user_ns(struct work_struct *work)
 		retire_userns_sysctls(ns);
 		key_free_user_ns(ns);
 		ns_free_inum(&ns->ns);
+		for (i = 0; i < UIDHASH_SZ; ++i)
+			uid_hash_remove_all(ns->uidhash_table + i);
 		kmem_cache_free(user_ns_cachep, ns);
 		dec_user_namespaces(ucounts);
 		ns = parent;


More information about the Devel mailing list