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

[Suren Baghdasaryan]

On Tue, Sep 29, 2026 at 10:20 AM Hao Ge <hao.ge@xxxxxxxxx> 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 <sashiko-bot@xxxxxxxxxx>
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hao Ge <hao.ge@xxxxxxxxx>
> ---
>  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
>