Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Percpu counters are permanently leaked when a codetag section falls 
back to regular module memory.
--

commit 67e030141d036ccecf172380286b7317fac48a51
Author: Hao Ge <[email protected]>

module: allocate codetag sections before the regular module layout

This patch moves codetag section allocation to occur before the regular
module layout and fails the load on allocation errors. However, on tag
area overflow, it gracefully falls back to allocating the codetag section
in regular module memory instead.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d0e1e0bd2ad06..da80f7c187921 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c

[ ... ]

> @@ -2951,9 +2906,48 @@ static bool blacklisted(const char *module_name)
>  }
>  core_param(module_blacklist, module_blacklist, charp, 0400);
>  
> +/*
> + * Allocate codetag sections separately. They are loaded into preallocated
> + * contiguous memory because they may still be used after the module is
> + * unloaded.
> + *
> + * If the separate allocation overflows, allocate the section normally
> + * so that the module can still be loaded.
> + */
> +static int allocate_codetag_sections(struct load_info *info)
> +{
> +     for (unsigned int i = 1; i < info->hdr->e_shnum; i++) {
> +             Elf_Shdr *shdr = &info->sechdrs[i];
> +             const char *sname = info->secstrings + shdr->sh_name;
> +             void *dest;
> +
> +             if (!codetag_needs_module_section(info->mod, sname, 
> shdr->sh_size))
> +                     continue;
> +
> +             dest = codetag_alloc_module_section(info->mod, sname, 
> shdr->sh_size,
> +                             arch_mod_section_prepend(info->mod, i), 
> shdr->sh_addralign);
> +             if (WARN_ON(!dest))
> +                     return -EINVAL;
> +             if (dest == ERR_PTR(-EAGAIN))
> +                     /* Allocate the section as a regular section. */
> +                     continue;

[Severity: High]
If reserve_module_tags() returns -EAGAIN due to reaching the tag addressable
limit, the module load proceeds by falling back to regular module memory.
However, does this cause a permanent percpu memory leak when the module is
eventually unloaded?

When the codetag section falls back to regular memory, the module is never
added to the mod_area_mt maple tree. Later, load_module() in mm/alloc_tag.c
unconditionally allocates percpu memory for all tags in the module:

mm/alloc_tag.c:load_module() {
    ...
    for (tag = start_tag; tag < stop_tag; tag++) {
        ...
        tag->counters = alloc_percpu(struct alloc_tag_counters);
    }
}

During unload, release_module_tags() attempts to free these counters. But
since the module was never added to the maple tree, it exits early before
cleaning up the counters:

mm/alloc_tag.c:release_module_tags() {
    ...
    mas_for_each_rev(&mas, val, 0)
        if (val == mod)
            break;

    if (!val) /* module not found */
        goto out;
    ...
}

Will this cause all percpu counters allocated for this module to be leaked,
potentially leading to percpu memory exhaustion over time?

> +             if (IS_ERR(dest))
> +                     return PTR_ERR(dest);
> +
> +             if (shdr->sh_type != SHT_NOBITS)
> +                     memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
> +             shdr->sh_addr = (unsigned long)dest;
> +             shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
> +             shdr->sh_entsize = SH_ENTSIZE_STANDALONE;
> +     }
> +
> +     return 0;
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to