On Tue, Sep 29, 2026 at 10:20 AM Hao Ge <[email protected]> wrote:
>
> 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);
Code looks correct to me but if you are changing alloc_tag_cttype to
be accessed under RCU, then you should declare it as __rcu and use
rcu_assign_pointer()/rcu_access_pointer()/rcu_dereference()/rcu_dereference_protected().
> + 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
>
