All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 = &sectioncheck[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 = &sectioncheck[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 = &sectioncheck[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 = &sectioncheck[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.