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 file creation is moved to the end of alloc_tag_init() as well.
If alloc_tag_init() fails with alloc_tag_cttype still NULL or an
error pointer, a concurrent reader of the leftover file would
dereference it in allocinfo_start() and panic.

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]>
---
 mm/alloc_tag.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 1ca0409b492b..cfa0fc84b68f 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>
 
@@ -591,6 +592,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 +608,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;
 }
 
@@ -1358,16 +1366,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;
        }
 
@@ -1375,10 +1377,16 @@ 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)) 
{
+               pr_err("Failed to create %s file\n", ALLOCINFO_FILE_NAME);
+               shutdown_mem_profiling(false);
+               return -ENOMEM;
+       }
+
        return 0;
 }
 module_init(alloc_tag_init);
-- 
2.25.1


Reply via email to