2022-05-30 23:01:29

by Masahiro Yamada

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

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 <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
---

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



2022-06-01 10:06:09

by Masahiro Yamada

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

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 <[email protected]>
---

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


2022-06-07 04:30:24

by Nick Desaulniers

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

On Mon, May 30, 2022 at 2:03 AM Masahiro Yamada <[email protected]> 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 <[email protected]>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>

> ---
>
> 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