Re: [PATCH v7 2/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `.data..percpu` section is incorrectly exported in sysfs with a dangling pointer to a freed temporary buffer.
--

commit 5082dc654c1b8850af405eef473b249c0d936de9
Author: Hao Ge <hao.ge@linux.dev>

module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

This commit introduces a new marker, SH_ENTSIZE_STANDALONE, to indicate
sections with a separate allocation. Instead of clearing the SHF_ALLOC
flag, it utilizes sh_entsize so that sections with separate allocations
like the percpu section can be correctly skipped during layout.

Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d0e1e0bd2ad06..b114b815187a9 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -2822,7 +2810,8 @@ static int move_module(struct module *mod, struct load_info *info)
>  		Elf_Shdr *shdr = &info->sechdrs[i];
>  		const char *sname;
>  
> -		if (!(shdr->sh_flags & SHF_ALLOC))
> +		if (!(shdr->sh_flags & SHF_ALLOC)
> +		    || shdr->sh_entsize == SH_ENTSIZE_STANDALONE)
>  			continue;
>  
>  		sname = info->secstrings + shdr->sh_name;
[ ... ]
> @@ -2967,8 +2957,13 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>  	if (err < 0)
>  		return ERR_PTR(err);
>  
> +	/* Repurpose sh_entsize to track where each section is allocated. */
> +	for (i = 0; i < info->hdr->e_shnum; i++)
> +		info->sechdrs[i].sh_entsize = ~0UL;
> +
>  	/* We will do a special allocation for per-cpu sections later. */
> -	info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
> +	if (info->index.pcpu)
> +		info->sechdrs[info->index.pcpu].sh_entsize = SH_ENTSIZE_STANDALONE;
>  
>  	/*
>  	 * Mark relevant sections as SHF_RO_AFTER_INIT so layout_sections() can

[Severity: Medium]
Because SHF_ALLOC is no longer cleared here, does this cause the
.data..percpu section to be incorrectly exported via sysfs?

With SHF_ALLOC remaining intact, sect_empty() in kernel/module/sysfs.c now
returns false for the percpu section, causing it to be processed instead of
skipped.

Additionally, because move_module() explicitly skips sections marked with
SH_ENTSIZE_STANDALONE, the sh_addr is never updated to the final allocated
memory.

This means when add_sect_attrs() exports the section:

kernel/module/sysfs.c:add_sect_attrs() {
    ...
	if (sect_empty(sec))
		continue;
    ...
	sattr->private = (void *)sec->sh_addr;
    ...
}

Will sattr->private end up storing an unmodified sh_addr pointing to the
freed temporary load_info buffer (info->hdr)?

This appears to leak a stale vmalloc pointer address to userspace when
reading the sysfs file for the section.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902081802.146145-1-hao.ge@linux.dev?part=2