* [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
* [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
* [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
* [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 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
* 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 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
* 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
* 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 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
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.