Re: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] An integer overflow in ALIGN() using unvalidated sh_addralign allows an attacker to spoof the memory type stored in sh_entsize, leading to an out-of-bounds read and kernel panic.
--

--- Patch [2]: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() ---
commit f621cb3d3b6809c9dd569a1f5e20bfb1e929c4d4
Author: Hao Ge <hao.ge@linux.dev>

module: move codetag section placement decision to layout_sections()

This commit moves the decision for codetag section placement to
layout_sections(), storing the result in sh_entsize using the new
MOD_MEM_CODETAG type. The move_module() function now relies on this stored
type instead of calling codetag_needs_module_section() again. This prevents
a race condition when profiling support is toggled during module load.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a6058..25056e87d0038 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -2815,11 +2812,10 @@ static int move_module(struct module *mod, struct load_info *info)
>  			continue;
>  
>  		sname = info->secstrings + shdr->sh_name;
> -		/*
> -		 * Load codetag sections separately as they might still be used
> -		 * after module unload.
> -		 */
> -		if (codetag_needs_module_section(mod, sname, shdr->sh_size)) {
> +
> +		enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT;
> +
> +		if (type == MOD_MEM_CODETAG) {

[Severity: High]
Can a maliciously crafted ELF module spoof the type by causing an integer
overflow in module_get_offset_and_type()?

If a module with two SHT_NOBITS sections is loaded, it might bypass ELF file
size validations. The first section could inflate mod->mem[MOD_DATA].size to
0x60000001. If the second section has sh_addralign = 0x10000000, it would
cause an integer overflow in module_get_offset_and_type():

kernel/module/main.c:module_get_offset_and_type() {
    ...
    offset = ALIGN(mod->mem[type].size, sechdr->sh_addralign ?: 1);
    ...
    return offset | mask;
}

The overflowed offset (0x70000000) places 7 (MOD_MEM_CODETAG) into the top 4
bits of sh_entsize. By setting the second section's sh_size to 0x90000000,
the attacker can wrap mod->mem[MOD_DATA].size back to 0, avoiding -ENOMEM
during module_memory_alloc().

If this spoofed type is trusted by move_module(), execution routes to
codetag_alloc_module_section():

lib/codetag.c:codetag_alloc_module_section() {
    ...
    const char *type_name = name + strlen(CODETAG_SECTION_PREFIX);
    ...
}

If a short-named section is routed here by the spoofed type, could blindly
advancing the name pointer cause an out-of-bounds read past the secstrings
vmalloc allocation and trigger a kernel panic?

>  			dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
>  					arch_mod_section_prepend(mod, i), shdr->sh_addralign);
>  			if (WARN_ON(!dest)) {

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