| Message ID | 20200210161852.842-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scripts/kallsyms: fix memory corruption caused by write over-run | expand |
On Tue 2020-02-11 01:18:52, Masahiro Yamada wrote: > scripts/kallsyms crashes because memcpy() writes one more byte than > allocated. > > Fixes: 8d60526999aa ("scripts/kallsyms: change table to store (strcut sym_entry *)") > Reported-by: youling257 <youling257@gmail.com> > Reported-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Tested-by: Pavel Machek <pavel@ucw.cz> Thanks! Pavel
Looks like len is already +1, maybe it shouldn't be? > len = strlen(name) + 1; > > - sym = malloc(sizeof(*sym) + len); > + sym = malloc(sizeof(*sym) + len + 1); Maybe strlcpy or if len wasn't incremented? > > - memcpy(sym_name(sym), name, len); > + strcpy(sym_name(sym), name); >
Hi. On Tue, Feb 11, 2020 at 11:36 AM Justin Capella <justincapella@gmail.com> wrote: > > Looks like len is already +1, maybe it shouldn't be? This increment is for storing one more character, 'type'. sym->sym[0] = type; > > len = strlen(name) + 1; > > > > - sym = malloc(sizeof(*sym) + len); > > + sym = malloc(sizeof(*sym) + len + 1); This increment is for the '\0' termination. So, malloc() needs to allocate: sizeof(*sym) + strlen(name) + 2. > > > Maybe strlcpy or if len wasn't incremented? > > > > > - memcpy(sym_name(sym), name, len); > > + strcpy(sym_name(sym), name); > >
On Tue, Feb 11, 2020 at 1:19 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > scripts/kallsyms crashes because memcpy() writes one more byte than > allocated. > > Fixes: 8d60526999aa ("scripts/kallsyms: change table to store (strcut sym_entry *)") > Reported-by: youling257 <youling257@gmail.com> > Reported-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- Applied. I will send a pull request shortly because many people are tripping over this bug. > scripts/kallsyms.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > index a566d8201b56..0133dfaaf352 100644 > --- a/scripts/kallsyms.c > +++ b/scripts/kallsyms.c > @@ -210,7 +210,7 @@ static struct sym_entry *read_symbol(FILE *in) > > len = strlen(name) + 1; > > - sym = malloc(sizeof(*sym) + len); > + sym = malloc(sizeof(*sym) + len + 1); > if (!sym) { > fprintf(stderr, "kallsyms failure: " > "unable to allocate required amount of memory\n"); > @@ -219,7 +219,7 @@ static struct sym_entry *read_symbol(FILE *in) > sym->addr = addr; > sym->len = len; > sym->sym[0] = type; > - memcpy(sym_name(sym), name, len); > + strcpy(sym_name(sym), name); > sym->percpu_absolute = 0; > > return sym; > -- > 2.17.1 >
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c index a566d8201b56..0133dfaaf352 100644 --- a/scripts/kallsyms.c +++ b/scripts/kallsyms.c @@ -210,7 +210,7 @@ static struct sym_entry *read_symbol(FILE *in) len = strlen(name) + 1; - sym = malloc(sizeof(*sym) + len); + sym = malloc(sizeof(*sym) + len + 1); if (!sym) { fprintf(stderr, "kallsyms failure: " "unable to allocate required amount of memory\n"); @@ -219,7 +219,7 @@ static struct sym_entry *read_symbol(FILE *in) sym->addr = addr; sym->len = len; sym->sym[0] = type; - memcpy(sym_name(sym), name, len); + strcpy(sym_name(sym), name); sym->percpu_absolute = 0; return sym;
scripts/kallsyms crashes because memcpy() writes one more byte than allocated. Fixes: 8d60526999aa ("scripts/kallsyms: change table to store (strcut sym_entry *)") Reported-by: youling257 <youling257@gmail.com> Reported-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/kallsyms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)