Skip to content

Commit 35adf9a

Browse files
ahunter6mcgrof
authored andcommitted
modules: Fix corruption of /proc/kallsyms
The commit 91fb02f ("module: Move kallsyms support into a separate file") changed from using strlcpy() to using strscpy() which created a buffer overflow. That happened because: 1) an incorrect value was passed as the buffer length 2) strscpy() (unlike strlcpy()) may copy beyond the length of the input string when copying word-by-word. The assumption was that because it was already known that the strings being copied would fit in the space available, it was not necessary to correctly set the buffer length. strscpy() breaks that assumption because although it will not touch bytes beyond the given buffer length it may write bytes beyond the input string length when writing word-by-word. The result of the buffer overflow is to corrupt the symbol type information that follows. e.g. $ sudo cat -v /proc/kallsyms | grep '\^' | head ffffffffc0615000 ^@ rfcomm_session_get [rfcomm] ffffffffc061c060 ^@ session_list [rfcomm] ffffffffc06150d0 ^@ rfcomm_send_frame [rfcomm] ffffffffc0615130 ^@ rfcomm_make_uih [rfcomm] ffffffffc07ed58d ^@ bnep_exit [bnep] ffffffffc07ec000 ^@ bnep_rx_control [bnep] ffffffffc07ec1a0 ^@ bnep_session [bnep] ffffffffc07e7000 ^@ input_leds_event [input_leds] ffffffffc07e9000 ^@ input_leds_handler [input_leds] ffffffffc07e7010 ^@ input_leds_disconnect [input_leds] Notably, the null bytes (represented above by ^@) can confuse tools. Fix by correcting the buffer length. Fixes: 91fb02f ("module: Move kallsyms support into a separate file") Signed-off-by: Adrian Hunter <[email protected]> Signed-off-by: Luis Chamberlain <[email protected]>
1 parent 1a0e93d commit 35adf9a

File tree

1 file changed

+12
-3
lines changed

1 file changed

+12
-3
lines changed

kernel/module/kallsyms.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
137137
info->symoffs = ALIGN(mod->data_layout.size, symsect->sh_addralign ?: 1);
138138
info->stroffs = mod->data_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
139139
mod->data_layout.size += strtab_size;
140+
/* Note add_kallsyms() computes strtab_size as core_typeoffs - stroffs */
140141
info->core_typeoffs = mod->data_layout.size;
141142
mod->data_layout.size += ndst * sizeof(char);
142143
mod->data_layout.size = strict_align(mod->data_layout.size);
@@ -169,6 +170,7 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
169170
Elf_Sym *dst;
170171
char *s;
171172
Elf_Shdr *symsec = &info->sechdrs[info->index.sym];
173+
unsigned long strtab_size;
172174

173175
/* Set up to point into init section. */
174176
mod->kallsyms = (void __rcu *)mod->init_layout.base +
@@ -190,19 +192,26 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
190192
mod->core_kallsyms.symtab = dst = mod->data_layout.base + info->symoffs;
191193
mod->core_kallsyms.strtab = s = mod->data_layout.base + info->stroffs;
192194
mod->core_kallsyms.typetab = mod->data_layout.base + info->core_typeoffs;
195+
strtab_size = info->core_typeoffs - info->stroffs;
193196
src = rcu_dereference_sched(mod->kallsyms)->symtab;
194197
for (ndst = i = 0; i < rcu_dereference_sched(mod->kallsyms)->num_symtab; i++) {
195198
rcu_dereference_sched(mod->kallsyms)->typetab[i] = elf_type(src + i, info);
196199
if (i == 0 || is_livepatch_module(mod) ||
197200
is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
198201
info->index.pcpu)) {
202+
ssize_t ret;
203+
199204
mod->core_kallsyms.typetab[ndst] =
200205
rcu_dereference_sched(mod->kallsyms)->typetab[i];
201206
dst[ndst] = src[i];
202207
dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
203-
s += strscpy(s,
204-
&rcu_dereference_sched(mod->kallsyms)->strtab[src[i].st_name],
205-
KSYM_NAME_LEN) + 1;
208+
ret = strscpy(s,
209+
&rcu_dereference_sched(mod->kallsyms)->strtab[src[i].st_name],
210+
strtab_size);
211+
if (ret < 0)
212+
break;
213+
s += ret + 1;
214+
strtab_size -= ret + 1;
206215
}
207216
}
208217
preempt_enable();

0 commit comments

Comments
 (0)