On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> 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@xxxxxxxxxx>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
