[PATCH v11 7/7] alloc_tag: fix the /proc/allocinfo lifecycle

[Hao Ge <hao.ge@linux.dev>]

shutdown_mem_profiling() calls remove_proc_entry() from
reserve_module_tags(), which runs under mod_lock held for write.
remove_proc_entry() waits for readers, and a reader takes mod_lock for
read in allocinfo_start():

  CPU0 (insmod)                      CPU1 (read /proc/allocinfo)
  ----------------                   ----------------------------
  reserve_module_tags()
    down_write(&mod_lock)  [held]
                                     use_pde()            [in_use++]
                                     allocinfo_start()
                                       down_read(&mod_lock)  <- blocks
    shutdown_mem_profiling()
      remove_proc_entry()
        wait for in_use == 0         <- blocks

Move remove_proc_entry() to a workqueue.

The deferred removal also affects alloc_tag_init(). The file is
created before the type, so on a failure it is still there with
alloc_tag_cttype NULL or an error pointer, and a reader panics in
allocinfo_start(). Create the file at the end of alloc_tag_init()
instead, a failed init leaves nothing behind.

If proc_create() fails, the codetag type and the module tags memory
leak. Call codetag_unregister_type() and free the memory.

alloc_tag_cttype can now be freed at runtime. alloc_tag_top_users()
reads it from __show_mem() without locks, read the pointer under
rcu_read_lock() and take mod_lock before dropping the RCU lock, the
type stays alive until then. The only caller never sleeps, drop the
can_sleep argument.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 include/linux/alloc_tag.h |  2 +-
 include/linux/codetag.h   |  2 ++
 lib/codetag.c             | 26 ++++++++++++++++++++
 mm/alloc_tag.c            | 52 +++++++++++++++++++++++++++------------
 mm/show_mem.c             |  2 +-
 5 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 7f2d80a59792..852dc10c00ee 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -81,7 +81,7 @@ struct codetag_bytes {
 	s64 bytes;
 };
 
-size_t alloc_tag_top_users(struct codetag_bytes *tags, size_t count, bool can_sleep);
+size_t alloc_tag_top_users(struct codetag_bytes *tags, size_t count);
 
 static inline struct alloc_tag *ct_to_alloc_tag(struct codetag *ct)
 {
diff --git a/include/linux/codetag.h b/include/linux/codetag.h
index a25a085c2df1..0c4e0337b474 100644
--- a/include/linux/codetag.h
+++ b/include/linux/codetag.h
@@ -87,6 +87,8 @@ void codetag_to_text(struct seq_buf *out, struct codetag *ct);
 struct codetag_type *
 codetag_register_type(const struct codetag_type_desc *desc);
 
+void codetag_unregister_type(struct codetag_type *cttype);
+
 #if defined(CONFIG_CODE_TAGGING) && defined(CONFIG_MODULES)
 
 bool codetag_needs_module_section(struct module *mod, const char *name,
diff --git a/lib/codetag.c b/lib/codetag.c
index a0b600720afc..46d0904b08b3 100644
--- a/lib/codetag.c
+++ b/lib/codetag.c
@@ -429,3 +429,29 @@ codetag_register_type(const struct codetag_type_desc *desc)
 
 	return cttype;
 }
+
+/**
+ * codetag_unregister_type - unregister a codetag type
+ * @cttype: the codetag type to unregister
+ *
+ * Undo codetag_register_type() and free @cttype. The caller must make
+ * sure no lockless reader still uses @cttype, e.g. clear the pointer
+ * to it and wait for an RCU grace period first.
+ */
+void __init codetag_unregister_type(struct codetag_type *cttype)
+{
+	struct codetag_module *cmod;
+	unsigned long id, tmp;
+
+	mutex_lock(&codetag_lock);
+	list_del(&cttype->link);
+	mutex_unlock(&codetag_lock);
+
+	down_write(&cttype->mod_lock);
+	idr_for_each_entry_ul(&cttype->mod_idr, cmod, tmp, id)
+		kfree(cmod);
+	idr_destroy(&cttype->mod_idr);
+	up_write(&cttype->mod_lock);
+
+	kfree(cttype);
+}
diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index ba8a651769e3..b9af5fe5bba2 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -15,6 +15,7 @@
 #include <linux/seq_file.h>
 #include <linux/string_choices.h>
 #include <linux/vmalloc.h>
+#include <linux/workqueue.h>
 #include <linux/kmemleak.h>
 #include <uapi/linux/alloc_tag.h>
 
@@ -484,22 +485,28 @@ static const struct proc_ops allocinfo_proc_ops = {
 #endif
 };
 
-size_t alloc_tag_top_users(struct codetag_bytes *tags, size_t count, bool can_sleep)
+size_t alloc_tag_top_users(struct codetag_bytes *tags, size_t count)
 {
 	struct codetag_iterator iter;
+	struct codetag_type *cttype;
 	struct codetag *ct;
 	struct codetag_bytes n;
 	unsigned int i, nr = 0;
+	bool locked;
 
-	if (IS_ERR_OR_NULL(alloc_tag_cttype))
+	rcu_read_lock();
+	cttype = READ_ONCE(alloc_tag_cttype);
+	if (IS_ERR_OR_NULL(cttype)) {
+		rcu_read_unlock();
 		return 0;
+	}
 
-	if (can_sleep)
-		codetag_lock_module_list(alloc_tag_cttype);
-	else if (!codetag_trylock_module_list(alloc_tag_cttype))
+	locked = codetag_trylock_module_list(cttype);
+	rcu_read_unlock();
+	if (!locked)
 		return 0;
 
-	iter = codetag_get_ct_iter(alloc_tag_cttype);
+	iter = codetag_get_ct_iter(cttype);
 	while ((ct = codetag_next_ct(&iter))) {
 		struct alloc_tag_counters counter = alloc_tag_read(ct_to_alloc_tag(ct));
 
@@ -520,7 +527,7 @@ size_t alloc_tag_top_users(struct codetag_bytes *tags, size_t count, bool can_sl
 		}
 	}
 
-	codetag_unlock_module_list(alloc_tag_cttype);
+	codetag_unlock_module_list(cttype);
 
 	return nr;
 }
@@ -591,6 +598,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
 	put_page_tag_ref(handle_new);
 }
 
+static void remove_allocinfo_file(struct work_struct *work)
+{
+	remove_proc_entry(ALLOCINFO_FILE_NAME, NULL);
+}
+
+static DECLARE_WORK(remove_allocinfo_work, remove_allocinfo_file);
+
 static void shutdown_mem_profiling(bool remove_file)
 {
 	if (mem_alloc_profiling_enabled())
@@ -600,7 +614,7 @@ static void shutdown_mem_profiling(bool remove_file)
 		return;
 
 	if (remove_file)
-		remove_proc_entry(ALLOCINFO_FILE_NAME, NULL);
+		schedule_work(&remove_allocinfo_work);
 	mem_profiling_support = false;
 }
 
@@ -1351,16 +1365,10 @@ static int __init alloc_tag_init(void)
 		return 0;
 	}
 
-	if (!proc_create(ALLOCINFO_FILE_NAME, 0400, NULL, &allocinfo_proc_ops)) {
-		pr_err("Failed to create %s file\n", ALLOCINFO_FILE_NAME);
-		shutdown_mem_profiling(false);
-		return -ENOMEM;
-	}
-
 	res = alloc_mod_tags_mem();
 	if (res) {
 		pr_err("Failed to reserve address space for module tags, errno = %d\n", res);
-		shutdown_mem_profiling(true);
+		shutdown_mem_profiling(false);
 		return res;
 	}
 
@@ -1368,10 +1376,22 @@ static int __init alloc_tag_init(void)
 	if (IS_ERR(alloc_tag_cttype)) {
 		pr_err("Allocation tags registration failed, errno = %pe\n", alloc_tag_cttype);
 		free_mod_tags_mem();
-		shutdown_mem_profiling(true);
+		shutdown_mem_profiling(false);
 		return PTR_ERR(alloc_tag_cttype);
 	}
 
+	if (!proc_create(ALLOCINFO_FILE_NAME, 0400, NULL, &allocinfo_proc_ops)) {
+		struct codetag_type *cttype = alloc_tag_cttype;
+
+		pr_err("Failed to create %s file\n", ALLOCINFO_FILE_NAME);
+		shutdown_mem_profiling(false);
+		WRITE_ONCE(alloc_tag_cttype, NULL);
+		synchronize_rcu();
+		codetag_unregister_type(cttype);
+		free_mod_tags_mem();
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 module_init(alloc_tag_init);
diff --git a/mm/show_mem.c b/mm/show_mem.c
index b938cbcd774a..a2e710404a48 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -439,7 +439,7 @@ void __show_mem(unsigned int filter, const nodemask_t *nodemask,
 		struct codetag_bytes tags[10];
 		size_t i, nr;
 
-		nr = alloc_tag_top_users(tags, ARRAY_SIZE(tags), false);
+		nr = alloc_tag_top_users(tags, ARRAY_SIZE(tags));
 		if (nr) {
 			pr_notice("Memory allocations (profiling is currently turned %s):\n",
 				mem_alloc_profiling_enabled() ? "on" : "off");
-- 
2.25.1