Thread (9 messages) flat view 9 messages, 3 authors, 18h ago

Re: [PATCH v3 2/3] module/kallsyms: Refactor current symbol filtering into is_kept_symbol()

From: sashiko-bot@kernel.org
Date: 2026-08-12 03:42:30

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] An attacker can use ELF relocations targeting the `.symtab` section to bypass early validation checks. This leads to heap buffer overflows, out-of-bounds reads, and information leaks during module loading.
--
--- Patch [2]: [PATCH v3 2/3] module/kallsyms: Refactor current symbol filtering into is_kept_symbol() ---
commit a466dcd66d6b608624bbdcca606a5281e2967615
Author: Tiezhu Yang [off-list ref]

module/kallsyms: Refactor current symbol filtering into is_kept_symbol()

Currently, the loops in layout_symtab() and add_kallsyms() duplicate the
exact same checks to determine whether a symbol should be preserved in
the module's core symbol table. This commit renames is_core_symbol() to
is_kept_symbol() and consolidates the checking logic.
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 8955cd42f20bc..efc73f66defd5 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -197,23 +199,21 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
 	strtab_size = info->core_typeoffs - info->stroffs;
 	src = kallsyms->symtab;
 	for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
+		ssize_t ret;
+
 		kallsyms->typetab[i] = elf_type(src + i, info);
-		if (i == 0 || is_livepatch_module(mod) ||
-		    is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
-				   info->index.pcpu)) {
-			ssize_t ret;
-
-			mod->core_kallsyms.typetab[ndst] =
-				kallsyms->typetab[i];
-			dst[ndst] = src[i];
-			dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
-			ret = strscpy(s, &kallsyms->strtab[src[i].st_name],
-				      strtab_size);
-			if (ret < 0)
-				break;
-			s += ret + 1;
-			strtab_size -= ret + 1;
-		}
+
+		if (!is_kept_symbol(mod, info, i, src + i))
+			continue;
+
+		mod->core_kallsyms.typetab[ndst] = kallsyms->typetab[i];
+		dst[ndst] = src[i];
+		dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
+		ret = strscpy(s, &kallsyms->strtab[src[i].st_name], strtab_size);
[Severity: High]
This isn't a bug introduced by this patch, but could an attacker use ELF
relocations targeting the .symtab section to bypass early validation checks
and trigger a heap buffer overflow or out-of-bounds read here?

During module loading, layout_symtab() adds the SHF_ALLOC flag to the
.symtab section and computes memory requirements based on the unmodified
symbols:

kernel/module/kallsyms.c:layout_symtab() {
    ...
    symsect->sh_flags |= SHF_ALLOC;
    ...
}

Later, apply_relocations() processes relocation sections. Because .symtab
now has the SHF_ALLOC flag, relocations targeting .symtab are applied,
modifying the symbol table in memory after bounds checks have completed:

kernel/module/main.c:apply_relocations() {
    ...
    if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC) &&
        (!infosec || infosec != info->index.pcpu))
        continue;
    ...
}

Finally, add_kallsyms() reads this modified symbol table without
re-validating it. If an attacker modifies st_name via relocations, it
bypasses the earlier elf_validity_cache_strtab() bounds check, causing
strscpy() to read out-of-bounds memory. Modifying st_shndx bypasses the
ndst calculation, causing out-of-bounds writes on mod->core_kallsyms.symtab
and typetab arrays.
+		if (ret < 0)
+			break;
+		s += ret + 1;
+		strtab_size -= ret + 1;
 	}
 
 	/* Set up to point into init section. */
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812032807.15034-1-yangtiezhu@loongson.cn?part=2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help