| Message ID | 20220523164626.858340-3-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() | expand |
On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > mod->name is set to the ELF filename with the suffix ".o" stripped. > > The current code calls strdup() and free() to manipulate the string, > but a simpler approach is to pass new_module() with the name length > subtracted by 2. > > Also, check if the passed filename ends with ".o" before stripping it. > > The current code blindly chops the suffix > > tmp[strlen(tmp) - 2] = '\0' > > but it will cause buffer under-run if strlen(tmp) < 2; > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/modpost.c | 25 ++++++++++++------------- > 1 file changed, 12 insertions(+), 13 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 843c64eebe8b..77c315dea1a3 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -172,11 +172,11 @@ static struct module *find_module(const char *modname) > return NULL; > } > > -static struct module *new_module(const char *modname) > +static struct module *new_module(const char *name, size_t namelen) > { > struct module *mod; > > - mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1)); > + mod = NOFAIL(malloc(sizeof(*mod) + namelen + 1)); > memset(mod, 0, sizeof(*mod)); > > INIT_LIST_HEAD(&mod->exported_symbols); > @@ -184,8 +184,9 @@ static struct module *new_module(const char *modname) > INIT_LIST_HEAD(&mod->missing_namespaces); > INIT_LIST_HEAD(&mod->imported_namespaces); > > - strcpy(mod->name, modname); > - mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0); > + memcpy(mod->name, name, namelen); > + mod->name[namelen] = '\0'; > + mod->is_vmlinux = (strcmp(mod->name, "vmlinux") == 0); > > /* > * Set mod->is_gpl_compatible to true by default. If MODULE_LICENSE() > @@ -2022,16 +2023,14 @@ static void read_symbols(const char *modname) > if (!parse_elf(&info, modname)) > return; > > - { > - char *tmp; > - > - /* strip trailing .o */ > - tmp = NOFAIL(strdup(modname)); > - tmp[strlen(tmp) - 2] = '\0'; > - mod = new_module(tmp); > - free(tmp); > + if (!strends(modname, ".o")) { > + error("%s: filename must be suffixed with .o\n", modname); > + return; > } > > + /* strip trailing .o */ > + mod = new_module(modname, strlen(modname) - strlen(".o")); > + > if (!mod->is_vmlinux) { > license = get_modinfo(&info, "license"); > if (!license) > @@ -2493,7 +2492,7 @@ static void read_dump(const char *fname) > > mod = find_module(modname); > if (!mod) { > - mod = new_module(modname); > + mod = new_module(modname, strlen(modname)); > mod->from_dump = true; > } > s = sym_add_exported(symname, mod, gpl_only); > -- > 2.32.0 >
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 843c64eebe8b..77c315dea1a3 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -172,11 +172,11 @@ static struct module *find_module(const char *modname) return NULL; } -static struct module *new_module(const char *modname) +static struct module *new_module(const char *name, size_t namelen) { struct module *mod; - mod = NOFAIL(malloc(sizeof(*mod) + strlen(modname) + 1)); + mod = NOFAIL(malloc(sizeof(*mod) + namelen + 1)); memset(mod, 0, sizeof(*mod)); INIT_LIST_HEAD(&mod->exported_symbols); @@ -184,8 +184,9 @@ static struct module *new_module(const char *modname) INIT_LIST_HEAD(&mod->missing_namespaces); INIT_LIST_HEAD(&mod->imported_namespaces); - strcpy(mod->name, modname); - mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0); + memcpy(mod->name, name, namelen); + mod->name[namelen] = '\0'; + mod->is_vmlinux = (strcmp(mod->name, "vmlinux") == 0); /* * Set mod->is_gpl_compatible to true by default. If MODULE_LICENSE() @@ -2022,16 +2023,14 @@ static void read_symbols(const char *modname) if (!parse_elf(&info, modname)) return; - { - char *tmp; - - /* strip trailing .o */ - tmp = NOFAIL(strdup(modname)); - tmp[strlen(tmp) - 2] = '\0'; - mod = new_module(tmp); - free(tmp); + if (!strends(modname, ".o")) { + error("%s: filename must be suffixed with .o\n", modname); + return; } + /* strip trailing .o */ + mod = new_module(modname, strlen(modname) - strlen(".o")); + if (!mod->is_vmlinux) { license = get_modinfo(&info, "license"); if (!license) @@ -2493,7 +2492,7 @@ static void read_dump(const char *fname) mod = find_module(modname); if (!mod) { - mod = new_module(modname); + mod = new_module(modname, strlen(modname)); mod->from_dump = true; } s = sym_add_exported(symname, mod, gpl_only);
mod->name is set to the ELF filename with the suffix ".o" stripped. The current code calls strdup() and free() to manipulate the string, but a simpler approach is to pass new_module() with the name length subtracted by 2. Also, check if the passed filename ends with ".o" before stripping it. The current code blindly chops the suffix tmp[strlen(tmp) - 2] = '\0' but it will cause buffer under-run if strlen(tmp) < 2; Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)