Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Systemic Use-After-Free (UAF) due to missing SHF_ALLOC validation across multiple subsystems. - [High] Integer overflow in module layout calculation leading to out-of-bounds reads. -- --- Patch [1]: [PATCH] lib/bug: ignore non-allocated module bug tables --- commit 2f65def59be64b4d6ec8eb77329b4e7bc9acac8c Author: Laxman Acharya Padhya <[email protected]> lib/bug: ignore non-allocated module bug tables This commit updates module_bug_finalize() to require the __bug_table section to have the SHF_ALLOC flag set before retaining its address. This prevents a malformed module from leaving the bug table pointing into the temporary module image, which is freed after loading. > diff --git a/lib/bug.c b/lib/bug.c > index 7c1c2c27f58e7..44966ead1e5fa 100644 > --- a/lib/bug.c > +++ b/lib/bug.c > @@ -94,7 +94,8 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const > Elf_Shdr *sechdrs, > /* Find the __bug_table section, if present */ > secstrings = (char *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; > for (i = 1; i < hdr->e_shnum; i++) { > - if (strcmp(secstrings+sechdrs[i].sh_name, "__bug_table")) > + if (strcmp(secstrings + sechdrs[i].sh_name, "__bug_table") || > + !(sechdrs[i].sh_flags & SHF_ALLOC)) > continue; [Severity: High] This is a pre-existing issue, but does this same vulnerability exist in other subsystems that look up sections by name without checking SHF_ALLOC? For example, in module_cfi_finalize() it assigns the section address to mod->kcfi_traps without verifying the SHF_ALLOC flag: kernel/cfi.c:module_cfi_finalize() { ... if (strcmp(secstrings + sechdrs[i].sh_name, "__kcfi_traps")) continue; mod->kcfi_traps = (s32 *)sechdrs[i].sh_addr; ... } Similarly, in unwind_module_init(), unvalidated ORC section addresses are saved into the module structure: arch/x86/kernel/unwind_orc.c:unwind_module_init() { ... mod->arch.orc_unwind_ip = orc_ip; mod->arch.orc_unwind = orc; ... } There is also module_init_ftrace_plt() which saves the unvalidated ftrace trampolines address retrieved via find_section(), which ignores flags: arch/arm64/include/asm/module.h:find_section() { ... if (strcmp(name, secstrs + s->sh_name) == 0) return s; ... } If a malformed module provides these sections without the SHF_ALLOC flag, would their sh_addr similarly point to the temporary module image buffer and cause a use-after-free when the buffer is freed? > mod->bug_table = (void *) sechdrs[i].sh_addr; > mod->num_bugs = sechdrs[i].sh_size / sizeof(struct bug_entry); [Severity: High] This isn't a bug introduced by this patch, but can an extremely large section size cause an integer overflow leading to an out-of-bounds read here? If a malformed module provides a section marked with SHT_NOBITS and SHF_ALLOC, elf_validity_cache_sechdrs() explicitly skips size validation: kernel/module/main.c:elf_validity_cache_sechdrs() { ... case SHT_NOBITS: /* No contents, offset/size don't mean anything */ continue; ... } Then, in module_get_offset_and_type(), adding a massive 64-bit sh_size to a 32-bit offset truncates the result: kernel/module/main.c:module_get_offset_and_type() { ... mod->mem[type].size = offset + sechdr->sh_size; ... } This wrap-around results in a small allocated memory layout size. However, since module_bug_finalize() uses the original un-truncated 64-bit sh_size to calculate mod->num_bugs, would a subsequent kernel warning or bug trigger iteration far beyond the actually allocated memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
