[PATCH 1/2] modpost: simplify mod->name allocation

[PATCH 1/2] modpost: simplify mod->name allocation

[Masahiro Yamada]

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'

It will cause buffer under-run if strlen(tmp) < 2;

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
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 9269735f85c5..c1558bacf717 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()
@@ -2017,16 +2018,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)
@@ -2488,7 +2487,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

[PATCH 2/2] modpost: use fnmatch() to simplify match()

[Masahiro Yamada]

Replace the own implementation for wildcard (glob) matching with
a function call to the library function, fnmatch().

Also, change the return type to 'bool'.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 74 ++++++++-----------------------------------
 1 file changed, 13 insertions(+), 61 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c1558bacf717..29d5a841e215 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -13,6 +13,7 @@
 
 #define _GNU_SOURCE
 #include <elf.h>
+#include <fnmatch.h>
 #include <stdio.h>
 #include <ctype.h>
 #include <string.h>
@@ -710,29 +711,6 @@ static char *get_modinfo(struct elf_info *info, const char *tag)
 	return get_next_modinfo(info, tag, NULL);
 }
 
-/**
- * Test if string s ends in string sub
- * return 0 if match
- **/
-static int strrcmp(const char *s, const char *sub)
-{
-	int slen, sublen;
-
-	if (!s || !sub)
-		return 1;
-
-	slen = strlen(s);
-	sublen = strlen(sub);
-
-	if ((slen == 0) || (sublen == 0))
-		return 1;
-
-	if (sublen > slen)
-		return 1;
-
-	return memcmp(s + slen - sublen, sub, sublen);
-}
-
 static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
 {
 	if (sym)
@@ -741,48 +719,22 @@ static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
 		return "(unknown)";
 }
 
-/* The pattern is an array of simple patterns.
- * "foo" will match an exact string equal to "foo"
- * "*foo" will match a string that ends with "foo"
- * "foo*" will match a string that begins with "foo"
- * "*foo*" will match a string that contains "foo"
+/*
+ * Check whether the 'string' argument matches one of the 'patterns',
+ * an array of shell wildcard patterns (glob).
+ *
+ * Return true is there is a match.
  */
-static int match(const char *sym, const char * const pat[])
+static bool match(const char *string, const char *const patterns[])
 {
-	const char *p;
-	while (*pat) {
-		const char *endp;
-
-		p = *pat++;
-		endp = p + strlen(p) - 1;
+	const char *pattern;
 
-		/* "*foo*" */
-		if (*p == '*' && *endp == '*') {
-			char *bare = NOFAIL(strndup(p + 1, strlen(p) - 2));
-			char *here = strstr(sym, bare);
-
-			free(bare);
-			if (here != NULL)
-				return 1;
-		}
-		/* "*foo" */
-		else if (*p == '*') {
-			if (strrcmp(sym, p + 1) == 0)
-				return 1;
-		}
-		/* "foo*" */
-		else if (*endp == '*') {
-			if (strncmp(sym, p, strlen(p) - 1) == 0)
-				return 1;
-		}
-		/* no wildcards */
-		else {
-			if (strcmp(p, sym) == 0)
-				return 1;
-		}
+	while ((pattern = *patterns++)) {
+		if (!fnmatch(pattern, string, 0))
+			return true;
 	}
-	/* no match */
-	return 0;
+
+	return false;
 }
 
 /* sections that we do not want to do full section mismatch check on */
-- 
2.32.0

Re: [PATCH 2/2] modpost: use fnmatch() to simplify match()

[Nick Desaulniers]

On Mon, May 30, 2022 at 2:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Replace the own implementation for wildcard (glob) matching with
> a function call to the library function, fnmatch().
>
> Also, change the return type to 'bool'.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/mod/modpost.c | 74 ++++++++-----------------------------------
>  1 file changed, 13 insertions(+), 61 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c1558bacf717..29d5a841e215 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -13,6 +13,7 @@
>
>  #define _GNU_SOURCE
>  #include <elf.h>
> +#include <fnmatch.h>
>  #include <stdio.h>
>  #include <ctype.h>
>  #include <string.h>
> @@ -710,29 +711,6 @@ static char *get_modinfo(struct elf_info *info, const char *tag)
>         return get_next_modinfo(info, tag, NULL);
>  }
>
> -/**
> - * Test if string s ends in string sub
> - * return 0 if match
> - **/
> -static int strrcmp(const char *s, const char *sub)
> -{
> -       int slen, sublen;
> -
> -       if (!s || !sub)
> -               return 1;
> -
> -       slen = strlen(s);
> -       sublen = strlen(sub);
> -
> -       if ((slen == 0) || (sublen == 0))
> -               return 1;
> -
> -       if (sublen > slen)
> -               return 1;
> -
> -       return memcmp(s + slen - sublen, sub, sublen);
> -}
> -
>  static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
>  {
>         if (sym)
> @@ -741,48 +719,22 @@ static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
>                 return "(unknown)";
>  }
>
> -/* The pattern is an array of simple patterns.
> - * "foo" will match an exact string equal to "foo"
> - * "*foo" will match a string that ends with "foo"
> - * "foo*" will match a string that begins with "foo"
> - * "*foo*" will match a string that contains "foo"
> +/*
> + * Check whether the 'string' argument matches one of the 'patterns',
> + * an array of shell wildcard patterns (glob).
> + *
> + * Return true is there is a match.
>   */
> -static int match(const char *sym, const char * const pat[])
> +static bool match(const char *string, const char *const patterns[])
>  {
> -       const char *p;
> -       while (*pat) {
> -               const char *endp;
> -
> -               p = *pat++;
> -               endp = p + strlen(p) - 1;
> +       const char *pattern;
>
> -               /* "*foo*" */
> -               if (*p == '*' && *endp == '*') {
> -                       char *bare = NOFAIL(strndup(p + 1, strlen(p) - 2));
> -                       char *here = strstr(sym, bare);
> -
> -                       free(bare);
> -                       if (here != NULL)
> -                               return 1;
> -               }
> -               /* "*foo" */
> -               else if (*p == '*') {
> -                       if (strrcmp(sym, p + 1) == 0)
> -                               return 1;
> -               }
> -               /* "foo*" */
> -               else if (*endp == '*') {
> -                       if (strncmp(sym, p, strlen(p) - 1) == 0)
> -                               return 1;
> -               }
> -               /* no wildcards */
> -               else {
> -                       if (strcmp(p, sym) == 0)
> -                               return 1;
> -               }
> +       while ((pattern = *patterns++)) {
> +               if (!fnmatch(pattern, string, 0))
> +                       return true;
>         }
> -       /* no match */
> -       return 0;
> +
> +       return false;
>  }
>
>  /* sections that we do not want to do full section mismatch check on */
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers