* [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() @ 2022-05-23 16:46 Masahiro Yamada 2022-05-23 16:46 ` [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() Masahiro Yamada ` (4 more replies) 0 siblings, 5 replies; 11+ messages in thread From: Masahiro Yamada @ 2022-05-23 16:46 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers The return value of is_arm_mapping_symbol() is unpredictable when "$" is passed in. strchr(3) says: The strchr() and strrchr() functions return a pointer to the matched character or NULL if the character is not found. The terminating null byte is considered part of the string, so that if c is specified as '\0', these functions return a pointer to the terminator. When str[1] is '\0', strchr("axtd", str[1]) is not NULL, and str[2] is referenced (i.e. buffer overrun). Test code --------- char str1[] = "abc"; char str2[] = "ab"; strcpy(str1, "$"); strcpy(str2, "$"); printf("test1: %d\n", is_arm_mapping_symbol(str1)); printf("test2: %d\n", is_arm_mapping_symbol(str2)); Result ------ test1: 0 test2: 1 Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 6f5c605ab0fb..845bc438ca49 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1179,7 +1179,8 @@ static int secref_whitelist(const struct sectioncheck *mismatch, static inline int is_arm_mapping_symbol(const char *str) { - return str[0] == '$' && strchr("axtd", str[1]) + return str[0] == '$' && + (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x') && (str[2] == '\0' || str[2] == '.'); } -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada @ 2022-05-23 16:46 ` Masahiro Yamada 2022-05-24 20:44 ` Nick Desaulniers 2022-05-23 16:46 ` [PATCH 3/5] modpost: simplify mod->name allocation Masahiro Yamada ` (3 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Masahiro Yamada @ 2022-05-23 16:46 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers check_sec_ref() does not use the first parameter 'mod'. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 845bc438ca49..843c64eebe8b 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1883,8 +1883,7 @@ static void section_rel(const char *modname, struct elf_info *elf, * to find all references to a section that reference a section that will * be discarded and warns about it. **/ -static void check_sec_ref(struct module *mod, const char *modname, - struct elf_info *elf) +static void check_sec_ref(const char *modname, struct elf_info *elf) { int i; Elf_Shdr *sechdrs = elf->sechdrs; @@ -2069,7 +2068,7 @@ static void read_symbols(const char *modname) sym_get_data(&info, sym)); } - check_sec_ref(mod, modname, &info); + check_sec_ref(modname, &info); if (!mod->is_vmlinux) { version = get_modinfo(&info, "version"); -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() 2022-05-23 16:46 ` [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() Masahiro Yamada @ 2022-05-24 20:44 ` Nick Desaulniers 2022-05-26 10:47 ` Masahiro Yamada 0 siblings, 1 reply; 11+ messages in thread From: Nick Desaulniers @ 2022-05-24 20:44 UTC (permalink / raw) To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > check_sec_ref() does not use the first parameter 'mod'. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Perhaps we could enable some -W flags for scripts/mod/? Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/modpost.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 845bc438ca49..843c64eebe8b 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1883,8 +1883,7 @@ static void section_rel(const char *modname, struct elf_info *elf, > * to find all references to a section that reference a section that will > * be discarded and warns about it. > **/ > -static void check_sec_ref(struct module *mod, const char *modname, > - struct elf_info *elf) > +static void check_sec_ref(const char *modname, struct elf_info *elf) > { > int i; > Elf_Shdr *sechdrs = elf->sechdrs; > @@ -2069,7 +2068,7 @@ static void read_symbols(const char *modname) > sym_get_data(&info, sym)); > } > > - check_sec_ref(mod, modname, &info); > + check_sec_ref(modname, &info); > > if (!mod->is_vmlinux) { > version = get_modinfo(&info, "version"); > -- > 2.32.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() 2022-05-24 20:44 ` Nick Desaulniers @ 2022-05-26 10:47 ` Masahiro Yamada 0 siblings, 0 replies; 11+ messages in thread From: Masahiro Yamada @ 2022-05-26 10:47 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, Linux Kernel Mailing List, Michal Marek On Wed, May 25, 2022 at 5:44 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > check_sec_ref() does not use the first parameter 'mod'. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Perhaps we could enable some -W flags for scripts/mod/? > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- Thanks for your advice. -Wunused-parameter found more. At least, we can clean up find_extable_entry_size(). HOSTCC scripts/mod/mk_elfconfig scripts/mod/mk_elfconfig.c: In function ‘main’: scripts/mod/mk_elfconfig.c:8:10: warning: unused parameter ‘argc’ [-Wunused-parameter] 8 | main(int argc, char **argv) | ~~~~^~~~ scripts/mod/mk_elfconfig.c:8:23: warning: unused parameter ‘argv’ [-Wunused-parameter] 8 | main(int argc, char **argv) | ~~~~~~~^~~~ CC scripts/mod/empty.o MKELF scripts/mod/elfconfig.h HOSTCC scripts/mod/modpost.o scripts/mod/modpost.c: In function ‘find_extable_entry_size’: scripts/mod/modpost.c:1538:55: warning: unused parameter ‘sec’ [-Wunused-parameter] 1538 | static void find_extable_entry_size(const char* const sec, const Elf_Rela* r) | ~~~~~~~~~~~~~~~~~~^~~ scripts/mod/modpost.c: In function ‘check_sec_ref’: scripts/mod/modpost.c:1885:42: warning: unused parameter ‘mod’ [-Wunused-parameter] 1885 | static void check_sec_ref(struct module *mod, const char *modname, | ~~~~~~~~~~~~~~~^~~ CC scripts/mod/devicetable-offsets.s HOSTCC scripts/mod/file2alias.o scripts/mod/file2alias.c: In function ‘do_hid_entry’: scripts/mod/file2alias.c:399:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 399 | static int do_hid_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ieee1394_entry’: scripts/mod/file2alias.c:417:42: warning: unused parameter ‘filename’ [-Wunused-parameter] 417 | static int do_ieee1394_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ccw_entry’: scripts/mod/file2alias.c:497:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 497 | static int do_ccw_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ap_entry’: scripts/mod/file2alias.c:520:36: warning: unused parameter ‘filename’ [-Wunused-parameter] 520 | static int do_ap_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_css_entry’: scripts/mod/file2alias.c:530:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 530 | static int do_css_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_serio_entry’: scripts/mod/file2alias.c:540:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 540 | static int do_serio_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_acpi_entry’: scripts/mod/file2alias.c:565:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 565 | static int do_acpi_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_pcmcia_entry’: scripts/mod/file2alias.c:682:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 682 | static int do_pcmcia_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_vio_entry’: scripts/mod/file2alias.c:718:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 718 | static int do_vio_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_input_entry’: scripts/mod/file2alias.c:752:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 752 | static int do_input_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_eisa_entry’: scripts/mod/file2alias.c:809:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 809 | static int do_eisa_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_parisc_entry’: scripts/mod/file2alias.c:821:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 821 | static int do_parisc_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_sdio_entry’: scripts/mod/file2alias.c:840:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 840 | static int do_sdio_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ssb_entry’: scripts/mod/file2alias.c:856:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 856 | static int do_ssb_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_bcma_entry’: scripts/mod/file2alias.c:872:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 872 | static int do_bcma_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_virtio_entry’: scripts/mod/file2alias.c:890:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 890 | static int do_virtio_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_vmbus_entry’: scripts/mod/file2alias.c:910:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 910 | static int do_vmbus_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_rpmsg_entry’: scripts/mod/file2alias.c:927:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 927 | static int do_rpmsg_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_i2c_entry’: scripts/mod/file2alias.c:937:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 937 | static int do_i2c_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_i3c_entry’: scripts/mod/file2alias.c:946:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 946 | static int do_i3c_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_spi_entry’: scripts/mod/file2alias.c:965:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 965 | static int do_spi_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_dmi_entry’: scripts/mod/file2alias.c:1006:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1006 | static int do_dmi_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_platform_entry’: scripts/mod/file2alias.c:1030:42: warning: unused parameter ‘filename’ [-Wunused-parameter] 1030 | static int do_platform_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_mdio_entry’: scripts/mod/file2alias.c:1038:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 1038 | static int do_mdio_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_zorro_entry’: scripts/mod/file2alias.c:1063:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 1063 | static int do_zorro_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_isapnp_entry’: scripts/mod/file2alias.c:1073:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 1073 | static int do_isapnp_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ipack_entry’: scripts/mod/file2alias.c:1088:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 1088 | static int do_ipack_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_mips_cdmm_entry’: scripts/mod/file2alias.c:1178:43: warning: unused parameter ‘filename’ [-Wunused-parameter] 1178 | static int do_mips_cdmm_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_x86cpu_entry’: scripts/mod/file2alias.c:1193:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 1193 | static int do_x86cpu_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_cpu_entry’: scripts/mod/file2alias.c:1212:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1212 | static int do_cpu_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_mei_entry’: scripts/mod/file2alias.c:1221:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1221 | static int do_mei_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_rio_entry’: scripts/mod/file2alias.c:1239:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1239 | static int do_rio_entry(const char *filename, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ulpi_entry’: scripts/mod/file2alias.c:1258:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 1258 | static int do_ulpi_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_hda_entry’: scripts/mod/file2alias.c:1270:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1270 | static int do_hda_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_sdw_entry’: scripts/mod/file2alias.c:1286:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1286 | static int do_sdw_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_fsl_mc_entry’: scripts/mod/file2alias.c:1304:40: warning: unused parameter ‘filename’ [-Wunused-parameter] 1304 | static int do_fsl_mc_entry(const char *filename, void *symval, | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_tbsvc_entry’: scripts/mod/file2alias.c:1315:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 1315 | static int do_tbsvc_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_typec_entry’: scripts/mod/file2alias.c:1339:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 1339 | static int do_typec_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_tee_entry’: scripts/mod/file2alias.c:1351:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1351 | static int do_tee_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_mhi_entry’: scripts/mod/file2alias.c:1387:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1387 | static int do_mhi_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ishtp_entry’: scripts/mod/file2alias.c:1395:39: warning: unused parameter ‘filename’ [-Wunused-parameter] 1395 | static int do_ishtp_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_auxiliary_entry’: scripts/mod/file2alias.c:1406:43: warning: unused parameter ‘filename’ [-Wunused-parameter] 1406 | static int do_auxiliary_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_ssam_entry’: scripts/mod/file2alias.c:1419:38: warning: unused parameter ‘filename’ [-Wunused-parameter] 1419 | static int do_ssam_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ scripts/mod/file2alias.c: In function ‘do_dfl_entry’: scripts/mod/file2alias.c:1437:37: warning: unused parameter ‘filename’ [-Wunused-parameter] 1437 | static int do_dfl_entry(const char *filename, void *symval, char *alias) | ~~~~~~~~~~~~^~~~~~~~ > > > > scripts/mod/modpost.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index 845bc438ca49..843c64eebe8b 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -1883,8 +1883,7 @@ static void section_rel(const char *modname, struct elf_info *elf, > > * to find all references to a section that reference a section that will > > * be discarded and warns about it. > > **/ > > -static void check_sec_ref(struct module *mod, const char *modname, > > - struct elf_info *elf) > > +static void check_sec_ref(const char *modname, struct elf_info *elf) > > { > > int i; > > Elf_Shdr *sechdrs = elf->sechdrs; > > @@ -2069,7 +2068,7 @@ static void read_symbols(const char *modname) > > sym_get_data(&info, sym)); > > } > > > > - check_sec_ref(mod, modname, &info); > > + check_sec_ref(modname, &info); > > > > if (!mod->is_vmlinux) { > > version = get_modinfo(&info, "version"); > > -- > > 2.32.0 > > > > > -- > Thanks, > ~Nick Desaulniers -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] modpost: simplify mod->name allocation 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada 2022-05-23 16:46 ` [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() Masahiro Yamada @ 2022-05-23 16:46 ` Masahiro Yamada 2022-05-24 20:47 ` Nick Desaulniers 2022-05-23 16:46 ` [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() Masahiro Yamada ` (2 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Masahiro Yamada @ 2022-05-23 16:46 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers 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(-) 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] modpost: simplify mod->name allocation 2022-05-23 16:46 ` [PATCH 3/5] modpost: simplify mod->name allocation Masahiro Yamada @ 2022-05-24 20:47 ` Nick Desaulniers 0 siblings, 0 replies; 11+ messages in thread From: Nick Desaulniers @ 2022-05-24 20:47 UTC (permalink / raw) To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek 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 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada 2022-05-23 16:46 ` [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() Masahiro Yamada 2022-05-23 16:46 ` [PATCH 3/5] modpost: simplify mod->name allocation Masahiro Yamada @ 2022-05-23 16:46 ` Masahiro Yamada 2022-05-24 20:49 ` Nick Desaulniers 2022-05-23 16:46 ` [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() Masahiro Yamada 2022-05-24 20:42 ` [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Nick Desaulniers 4 siblings, 1 reply; 11+ messages in thread From: Masahiro Yamada @ 2022-05-23 16:46 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers Move ARRAY_SIZE() from file2alias.c to modpost.h to reuse it in section_mismatch(). Also, move the variable 'check' inside the for-loop. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/file2alias.c | 2 -- scripts/mod/modpost.c | 7 +++---- scripts/mod/modpost.h | 3 +++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 5258247d78ac..e8a9c6816fec 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -734,8 +734,6 @@ static int do_vio_entry(const char *filename, void *symval, return 1; } -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) - static void do_input(char *alias, kernel_ulong_t *arr, unsigned int min, unsigned int max) { diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 77c315dea1a3..48a18b59f908 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1049,8 +1049,6 @@ static const struct sectioncheck *section_mismatch( const char *fromsec, const char *tosec) { int i; - int elems = sizeof(sectioncheck) / sizeof(struct sectioncheck); - const struct sectioncheck *check = §ioncheck[0]; /* * The target section could be the SHT_NUL section when we're @@ -1061,14 +1059,15 @@ static const struct sectioncheck *section_mismatch( if (*tosec == '\0') return NULL; - for (i = 0; i < elems; i++) { + for (i = 0; i < ARRAY_SIZE(sectioncheck); i++) { + const struct sectioncheck *check = §ioncheck[i]; + if (match(fromsec, check->fromsec)) { if (check->bad_tosec[0] && match(tosec, check->bad_tosec)) return check; if (check->good_tosec[0] && !match(tosec, check->good_tosec)) return check; } - check++; } return NULL; } diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index d9daeff07b83..044bdfb894b7 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -97,6 +97,9 @@ static inline void __endian(const void *src, void *dest, unsigned int size) #endif #define NOFAIL(ptr) do_nofail((ptr), #ptr) + +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) + void *do_nofail(void *ptr, const char *expr); struct buffer { -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() 2022-05-23 16:46 ` [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() Masahiro Yamada @ 2022-05-24 20:49 ` Nick Desaulniers 0 siblings, 0 replies; 11+ messages in thread From: Nick Desaulniers @ 2022-05-24 20:49 UTC (permalink / raw) To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Move ARRAY_SIZE() from file2alias.c to modpost.h to reuse it in > section_mismatch(). > > Also, move the variable 'check' inside the for-loop. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/file2alias.c | 2 -- > scripts/mod/modpost.c | 7 +++---- > scripts/mod/modpost.h | 3 +++ > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > index 5258247d78ac..e8a9c6816fec 100644 > --- a/scripts/mod/file2alias.c > +++ b/scripts/mod/file2alias.c > @@ -734,8 +734,6 @@ static int do_vio_entry(const char *filename, void *symval, > return 1; > } > > -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > - > static void do_input(char *alias, > kernel_ulong_t *arr, unsigned int min, unsigned int max) > { > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 77c315dea1a3..48a18b59f908 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1049,8 +1049,6 @@ static const struct sectioncheck *section_mismatch( > const char *fromsec, const char *tosec) > { > int i; > - int elems = sizeof(sectioncheck) / sizeof(struct sectioncheck); > - const struct sectioncheck *check = §ioncheck[0]; > > /* > * The target section could be the SHT_NUL section when we're > @@ -1061,14 +1059,15 @@ static const struct sectioncheck *section_mismatch( > if (*tosec == '\0') > return NULL; > > - for (i = 0; i < elems; i++) { > + for (i = 0; i < ARRAY_SIZE(sectioncheck); i++) { > + const struct sectioncheck *check = §ioncheck[i]; > + > if (match(fromsec, check->fromsec)) { > if (check->bad_tosec[0] && match(tosec, check->bad_tosec)) > return check; > if (check->good_tosec[0] && !match(tosec, check->good_tosec)) > return check; > } > - check++; > } > return NULL; > } > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > index d9daeff07b83..044bdfb894b7 100644 > --- a/scripts/mod/modpost.h > +++ b/scripts/mod/modpost.h > @@ -97,6 +97,9 @@ static inline void __endian(const void *src, void *dest, unsigned int size) > #endif > > #define NOFAIL(ptr) do_nofail((ptr), #ptr) > + > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > + > void *do_nofail(void *ptr, const char *expr); > > struct buffer { > -- > 2.32.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada ` (2 preceding siblings ...) 2022-05-23 16:46 ` [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() Masahiro Yamada @ 2022-05-23 16:46 ` Masahiro Yamada 2022-05-24 20:51 ` Nick Desaulniers 2022-05-24 20:42 ` [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Nick Desaulniers 4 siblings, 1 reply; 11+ messages in thread From: Masahiro Yamada @ 2022-05-23 16:46 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers if ((addr - sym->st_value) < distance) { distance = addr - sym->st_value; near = sym; } else if ((addr - sym->st_value) == distance) { near = sym; } is equivalent to: if ((addr - sym->st_value) <= distance) { distance = addr - sym->st_value; near = sym; } (The else-if part can overwrite 'distance' with the same value). Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 48a18b59f908..8c8d2a4bc0b0 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1270,13 +1270,9 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr, continue; if (!is_valid_name(elf, sym)) continue; - if (sym->st_value <= addr) { - if ((addr - sym->st_value) < distance) { - distance = addr - sym->st_value; - near = sym; - } else if ((addr - sym->st_value) == distance) { - near = sym; - } + if (sym->st_value <= addr && addr - sym->st_value <= distance) { + distance = addr - sym->st_value; + near = sym; } } return near; -- 2.32.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() 2022-05-23 16:46 ` [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() Masahiro Yamada @ 2022-05-24 20:51 ` Nick Desaulniers 0 siblings, 0 replies; 11+ messages in thread From: Nick Desaulniers @ 2022-05-24 20:51 UTC (permalink / raw) To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > if ((addr - sym->st_value) < distance) { > distance = addr - sym->st_value; > near = sym; > } else if ((addr - sym->st_value) == distance) { > near = sym; > } > > is equivalent to: > > if ((addr - sym->st_value) <= distance) { > distance = addr - sym->st_value; > near = sym; > } > > (The else-if part can overwrite 'distance' with the same value). > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Sure, thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/modpost.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 48a18b59f908..8c8d2a4bc0b0 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1270,13 +1270,9 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr, > continue; > if (!is_valid_name(elf, sym)) > continue; > - if (sym->st_value <= addr) { > - if ((addr - sym->st_value) < distance) { > - distance = addr - sym->st_value; > - near = sym; > - } else if ((addr - sym->st_value) == distance) { > - near = sym; > - } > + if (sym->st_value <= addr && addr - sym->st_value <= distance) { > + distance = addr - sym->st_value; > + near = sym; > } > } > return near; > -- > 2.32.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada ` (3 preceding siblings ...) 2022-05-23 16:46 ` [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() Masahiro Yamada @ 2022-05-24 20:42 ` Nick Desaulniers 4 siblings, 0 replies; 11+ messages in thread From: Nick Desaulniers @ 2022-05-24 20:42 UTC (permalink / raw) To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The return value of is_arm_mapping_symbol() is unpredictable when > "$" is passed in. > > strchr(3) says: > The strchr() and strrchr() functions return a pointer to the matched > character or NULL if the character is not found. The terminating null > byte is considered part of the string, so that if c is specified as > '\0', these functions return a pointer to the terminator. > > When str[1] is '\0', strchr("axtd", str[1]) is not NULL, and str[2] is > referenced (i.e. buffer overrun). > > Test code > --------- > > char str1[] = "abc"; > char str2[] = "ab"; > > strcpy(str1, "$"); > strcpy(str2, "$"); > > printf("test1: %d\n", is_arm_mapping_symbol(str1)); > printf("test2: %d\n", is_arm_mapping_symbol(str2)); > > Result > ------ > > test1: 0 > test2: 1 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> I guess this is shorter than a call to strlen then conditional call to strchr. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/modpost.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 6f5c605ab0fb..845bc438ca49 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1179,7 +1179,8 @@ static int secref_whitelist(const struct sectioncheck *mismatch, > > static inline int is_arm_mapping_symbol(const char *str) > { > - return str[0] == '$' && strchr("axtd", str[1]) > + return str[0] == '$' && > + (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x') > && (str[2] == '\0' || str[2] == '.'); > } > > -- > 2.32.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-05-26 10:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-23 16:46 [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Masahiro Yamada 2022-05-23 16:46 ` [PATCH 2/5] modpost: remove the unused argument of check_sec_ref() Masahiro Yamada 2022-05-24 20:44 ` Nick Desaulniers 2022-05-26 10:47 ` Masahiro Yamada 2022-05-23 16:46 ` [PATCH 3/5] modpost: simplify mod->name allocation Masahiro Yamada 2022-05-24 20:47 ` Nick Desaulniers 2022-05-23 16:46 ` [PATCH 4/5] modpost: reuse ARRAY_SIZE() macro for section_mismatch() Masahiro Yamada 2022-05-24 20:49 ` Nick Desaulniers 2022-05-23 16:46 ` [PATCH 5/5] modpost: squash if...else if in find_elf_symbol2() Masahiro Yamada 2022-05-24 20:51 ` Nick Desaulniers 2022-05-24 20:42 ` [PATCH 1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() Nick Desaulniers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.