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 <[email protected]>
Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: [email protected]
Signed-off-by: Hao Ge <[email protected]>
---
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
