* [PATCH] modules: Fix corruption of /proc/kallsyms
@ 2022-07-01 9:44 Adrian Hunter
2022-07-01 21:40 ` Luis Chamberlain
2022-07-11 17:37 ` Aaron Tomlin
0 siblings, 2 replies; 5+ messages in thread
From: Adrian Hunter @ 2022-07-01 9:44 UTC (permalink / raw)
To: Luis Chamberlain, Aaron Tomlin
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
The commit 91fb02f31505 ("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: 91fb02f31505 ("module: Move kallsyms support into a separate file")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
kernel/module/kallsyms.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 3e11523bc6f6..18c23545b984 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -137,6 +137,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
info->symoffs = ALIGN(mod->data_layout.size, symsect->sh_addralign ?: 1);
info->stroffs = mod->data_layout.size = info->symoffs + ndst * sizeof(Elf_Sym);
mod->data_layout.size += strtab_size;
+ /* Note add_kallsyms() computes strtab_size as core_typeoffs - stroffs */
info->core_typeoffs = mod->data_layout.size;
mod->data_layout.size += ndst * sizeof(char);
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)
Elf_Sym *dst;
char *s;
Elf_Shdr *symsec = &info->sechdrs[info->index.sym];
+ unsigned long strtab_size;
/* Set up to point into init section. */
mod->kallsyms = (void __rcu *)mod->init_layout.base +
@@ -190,19 +192,26 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
mod->core_kallsyms.symtab = dst = mod->data_layout.base + info->symoffs;
mod->core_kallsyms.strtab = s = mod->data_layout.base + info->stroffs;
mod->core_kallsyms.typetab = mod->data_layout.base + info->core_typeoffs;
+ strtab_size = info->core_typeoffs - info->stroffs;
src = rcu_dereference_sched(mod->kallsyms)->symtab;
for (ndst = i = 0; i < rcu_dereference_sched(mod->kallsyms)->num_symtab; i++) {
rcu_dereference_sched(mod->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] =
rcu_dereference_sched(mod->kallsyms)->typetab[i];
dst[ndst] = src[i];
dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
- s += strscpy(s,
- &rcu_dereference_sched(mod->kallsyms)->strtab[src[i].st_name],
- KSYM_NAME_LEN) + 1;
+ ret = strscpy(s,
+ &rcu_dereference_sched(mod->kallsyms)->strtab[src[i].st_name],
+ strtab_size);
+ if (ret < 0)
+ break;
+ s += ret + 1;
+ strtab_size -= ret + 1;
}
}
preempt_enable();
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] modules: Fix corruption of /proc/kallsyms
2022-07-01 9:44 [PATCH] modules: Fix corruption of /proc/kallsyms Adrian Hunter
@ 2022-07-01 21:40 ` Luis Chamberlain
2022-07-11 7:48 ` Adrian Hunter
2022-07-11 17:37 ` Aaron Tomlin
1 sibling, 1 reply; 5+ messages in thread
From: Luis Chamberlain @ 2022-07-01 21:40 UTC (permalink / raw)
To: Adrian Hunter
Cc: Aaron Tomlin, Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
On Fri, Jul 01, 2022 at 12:44:03PM +0300, Adrian Hunter wrote:
> The commit 91fb02f31505 ("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: 91fb02f31505 ("module: Move kallsyms support into a separate file")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Queued up thanks!
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] modules: Fix corruption of /proc/kallsyms
2022-07-01 21:40 ` Luis Chamberlain
@ 2022-07-11 7:48 ` Adrian Hunter
2022-07-11 16:02 ` Luis Chamberlain
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2022-07-11 7:48 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Aaron Tomlin, Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
On 2/07/22 00:40, Luis Chamberlain wrote:
> On Fri, Jul 01, 2022 at 12:44:03PM +0300, Adrian Hunter wrote:
>> The commit 91fb02f31505 ("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: 91fb02f31505 ("module: Move kallsyms support into a separate file")
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Queued up thanks!
>
> Luis
Thanks for processing this.
I notice it is -rc6 and I do not see it in Linus' tree. This is a fix
for a regression, shouldn't it be included in 5.19?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] modules: Fix corruption of /proc/kallsyms
2022-07-11 7:48 ` Adrian Hunter
@ 2022-07-11 16:02 ` Luis Chamberlain
0 siblings, 0 replies; 5+ messages in thread
From: Luis Chamberlain @ 2022-07-11 16:02 UTC (permalink / raw)
To: Adrian Hunter
Cc: Aaron Tomlin, Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
On Mon, Jul 11, 2022 at 10:48:16AM +0300, Adrian Hunter wrote:
> On 2/07/22 00:40, Luis Chamberlain wrote:
> > On Fri, Jul 01, 2022 at 12:44:03PM +0300, Adrian Hunter wrote:
> >> The commit 91fb02f31505 ("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: 91fb02f31505 ("module: Move kallsyms support into a separate file")
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >
> > Queued up thanks!
> >
> > Luis
>
> Thanks for processing this.
>
> I notice it is -rc6 and I do not see it in Linus' tree. This is a fix
> for a regression, shouldn't it be included in 5.19?
Yeah sorry this will get to Linus today.
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] modules: Fix corruption of /proc/kallsyms
2022-07-01 9:44 [PATCH] modules: Fix corruption of /proc/kallsyms Adrian Hunter
2022-07-01 21:40 ` Luis Chamberlain
@ 2022-07-11 17:37 ` Aaron Tomlin
1 sibling, 0 replies; 5+ messages in thread
From: Aaron Tomlin @ 2022-07-11 17:37 UTC (permalink / raw)
To: Adrian Hunter
Cc: Luis Chamberlain, Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
On Fri 2022-07-01 12:44 +0300, Adrian Hunter wrote:
> The commit 91fb02f31505 ("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: 91fb02f31505 ("module: Move kallsyms support into a separate file")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> kernel/module/kallsyms.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
Hi Adrian,
Thanks for following up!
Kind regards,
--
Aaron Tomlin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-11 17:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01 9:44 [PATCH] modules: Fix corruption of /proc/kallsyms Adrian Hunter
2022-07-01 21:40 ` Luis Chamberlain
2022-07-11 7:48 ` Adrian Hunter
2022-07-11 16:02 ` Luis Chamberlain
2022-07-11 17:37 ` Aaron Tomlin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).