diff mbox series

[4/6] modpost: stop symbol preloading for modversion CRC

Message ID 20191114174226.7201-4-yamada.masahiro@socionext.com
State New
Headers show
Series [1/6] modpost: add a helper to get data pointed by a symbol | expand

Commit Message

Masahiro Yamada Nov. 14, 2019, 5:42 p.m. UTC 
It is complicated to add mocked-up symbols to pre-handle CRC.
Handle CRC after all the export symbols in the relevant module
are registered.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 scripts/mod/modpost.c | 64 +++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

-- 
2.17.1

Comments

Masahiro Yamada Nov. 23, 2019, 6:42 a.m. UTC | #1 
On Fri, Nov 15, 2019 at 2:42 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>

> It is complicated to add mocked-up symbols to pre-handle CRC.

> Handle CRC after all the export symbols in the relevant module

> are registered.

>

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

>

>  scripts/mod/modpost.c | 64 +++++++++++++++++++++++--------------------

>  1 file changed, 35 insertions(+), 29 deletions(-)

>

> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c

> index 6735ae3da4c2..73bdf27c41fe 100644

> --- a/scripts/mod/modpost.c

> +++ b/scripts/mod/modpost.c

> @@ -169,7 +169,7 @@ struct symbol {

>         unsigned int vmlinux:1;    /* 1 if symbol is defined in vmlinux */

>         unsigned int kernel:1;     /* 1 if symbol is from kernel

>                                     *  (only for external modules) **/

> -       unsigned int preloaded:1;  /* 1 if symbol from Module.symvers, or crc */

> +       unsigned int preloaded:1;  /* 1 if symbol from Module.symvers */

>         unsigned int is_static:1;  /* 1 if symbol is not global */

>         enum export  export;       /* Type of export */

>         char name[0];

> @@ -410,15 +410,15 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,

>         return s;

>  }

>

> -static void sym_update_crc(const char *name, struct module *mod,

> -                          unsigned int crc, enum export export)

> +static void sym_set_crc(const char *name, const struct module *mod,

> +                       unsigned int crc)

>  {

>         struct symbol *s = find_symbol(name);

>

>         if (!s) {

> -               s = new_symbol(name, mod, export);

> -               /* Don't complain when we find it later. */

> -               s->preloaded = 1;

> +               warn("%s: '__crc_%s' is invalid use. __crc_* is reserved for modversion\n",

> +                    mod->name, name);


I notice this can produce false positive warnings.

ARCH=arm allyesconfig produces the following:

WARNING: vmlinux: '__crc_ccitt_veneer' is invalid use. __crc_* is
reserved for modversion
WARNING: vmlinux: '__crc_ccitt_veneer' is invalid use. __crc_* is
reserved for modversion
WARNING: vmlinux: '__crc_ccitt_veneer' is invalid use. __crc_* is
reserved for modversion
WARNING: vmlinux: '__crc_itu_t_veneer' is invalid use. __crc_* is
reserved for modversion
WARNING: vmlinux: '__crc_itu_t_veneer' is invalid use. __crc_* is
reserved for modversion
WARNING: vmlinux: '__crc_itu_t_veneer' is invalid use. __crc_* is
reserved for modversion


The ARM compiler inserts veneers automatically.

I will remove this warn(), and add commit messages as follows:

    In some cases, I see atand-alone __crc_* without __ksymtab_*.
    For example, ARCH=arm allyesconfig produces __crc_ccitt_veneer and
    __crc_itu_t_veneer. I guess they come from crc_ccitt, crc_itu_t,
    respectively. Since __*_veneer are auto-generated symbols, just
    ignore them.



> +               return;

>         }

>         s->crc = crc;

>         s->crc_valid = 1;

> @@ -683,12 +683,34 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)

>         return 0;

>  }

>

> +static void handle_modversion(const struct module *mod,

> +                             const struct elf_info *info,

> +                             const Elf_Sym *sym, const char *symname)

> +{

> +       unsigned int crc;

> +

> +       if (sym->st_shndx == SHN_UNDEF) {

> +               warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n",

> +                    symname, mod->name, is_vmlinux(mod->name) ? "":".ko");

> +               return;

> +       }

> +

> +       if (sym->st_shndx == SHN_ABS) {

> +               crc = sym->st_value;

> +       } else {

> +               unsigned int *crcp;

> +

> +               /* symbol points to the CRC in the ELF object */

> +               crcp = sym_get_data(info, sym);

> +               crc = TO_NATIVE(*crcp);

> +       }

> +       sym_set_crc(symname, mod, crc);

> +}

> +

>  static void handle_symbol(struct module *mod, struct elf_info *info,

>                           const Elf_Sym *sym, const char *symname)

>  {

> -       unsigned int crc;

>         enum export export;

> -       bool is_crc = false;

>         const char *name;

>

>         if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&

> @@ -697,21 +719,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info,

>         else

>                 export = export_from_sec(info, get_secindex(info, sym));

>

> -       /* CRC'd symbol */

> -       if (strstarts(symname, "__crc_")) {

> -               is_crc = true;

> -               crc = (unsigned int) sym->st_value;

> -               if (sym->st_shndx != SHN_UNDEF && sym->st_shndx != SHN_ABS) {

> -                       unsigned int *crcp;

> -

> -                       /* symbol points to the CRC in the ELF object */

> -                       crcp = sym_get_data(info, sym);

> -                       crc = TO_NATIVE(*crcp);

> -               }

> -               sym_update_crc(symname + strlen("__crc_"), mod, crc,

> -                               export);

> -       }

> -

>         switch (sym->st_shndx) {

>         case SHN_COMMON:

>                 if (strstarts(symname, "__gnu_lto_")) {

> @@ -746,11 +753,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info,

>                 }

>  #endif

>

> -               if (is_crc) {

> -                       const char *e = is_vmlinux(mod->name) ?"":".ko";

> -                       warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n",

> -                            symname + strlen("__crc_"), mod->name, e);

> -               }

>                 mod->unres = alloc_symbol(symname,

>                                           ELF_ST_BIND(sym->st_info) == STB_WEAK,

>                                           mod->unres);

> @@ -2063,6 +2065,10 @@ static void read_symbols(const char *modname)

>                         sym_update_namespace(symname + strlen("__kstrtabns_"),

>                                              namespace_from_kstrtabns(&info,

>                                                                       sym));

> +

> +               if (strstarts(symname, "__crc_"))

> +                       handle_modversion(mod, &info, sym,

> +                                         symname + strlen("__crc_"));

>         }

>

>         // check for static EXPORT_SYMBOL_* functions && global vars

> @@ -2476,7 +2482,7 @@ static void read_dump(const char *fname, unsigned int kernel)

>                 s->kernel    = kernel;

>                 s->preloaded = 1;

>                 s->is_static = 0;

> -               sym_update_crc(symname, mod, crc, export_no(export));

> +               sym_set_crc(symname, mod, crc);

>                 sym_update_namespace(symname, namespace);

>         }

>         release_file(file, size);

> --

> 2.17.1

>



-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6735ae3da4c2..73bdf27c41fe 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -169,7 +169,7 @@  struct symbol {
 	unsigned int vmlinux:1;    /* 1 if symbol is defined in vmlinux */
 	unsigned int kernel:1;     /* 1 if symbol is from kernel
 				    *  (only for external modules) **/
-	unsigned int preloaded:1;  /* 1 if symbol from Module.symvers, or crc */
+	unsigned int preloaded:1;  /* 1 if symbol from Module.symvers */
 	unsigned int is_static:1;  /* 1 if symbol is not global */
 	enum export  export;       /* Type of export */
 	char name[0];
@@ -410,15 +410,15 @@  static struct symbol *sym_add_exported(const char *name, struct module *mod,
 	return s;
 }
 
-static void sym_update_crc(const char *name, struct module *mod,
-			   unsigned int crc, enum export export)
+static void sym_set_crc(const char *name, const struct module *mod,
+			unsigned int crc)
 {
 	struct symbol *s = find_symbol(name);
 
 	if (!s) {
-		s = new_symbol(name, mod, export);
-		/* Don't complain when we find it later. */
-		s->preloaded = 1;
+		warn("%s: '__crc_%s' is invalid use. __crc_* is reserved for modversion\n",
+		     mod->name, name);
+		return;
 	}
 	s->crc = crc;
 	s->crc_valid = 1;
@@ -683,12 +683,34 @@  static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 	return 0;
 }
 
+static void handle_modversion(const struct module *mod,
+			      const struct elf_info *info,
+			      const Elf_Sym *sym, const char *symname)
+{
+	unsigned int crc;
+
+	if (sym->st_shndx == SHN_UNDEF) {
+		warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n",
+		     symname, mod->name, is_vmlinux(mod->name) ? "":".ko");
+		return;
+	}
+
+	if (sym->st_shndx == SHN_ABS) {
+		crc = sym->st_value;
+	} else {
+		unsigned int *crcp;
+
+		/* symbol points to the CRC in the ELF object */
+		crcp = sym_get_data(info, sym);
+		crc = TO_NATIVE(*crcp);
+	}
+	sym_set_crc(symname, mod, crc);
+}
+
 static void handle_symbol(struct module *mod, struct elf_info *info,
 			  const Elf_Sym *sym, const char *symname)
 {
-	unsigned int crc;
 	enum export export;
-	bool is_crc = false;
 	const char *name;
 
 	if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
@@ -697,21 +719,6 @@  static void handle_symbol(struct module *mod, struct elf_info *info,
 	else
 		export = export_from_sec(info, get_secindex(info, sym));
 
-	/* CRC'd symbol */
-	if (strstarts(symname, "__crc_")) {
-		is_crc = true;
-		crc = (unsigned int) sym->st_value;
-		if (sym->st_shndx != SHN_UNDEF && sym->st_shndx != SHN_ABS) {
-			unsigned int *crcp;
-
-			/* symbol points to the CRC in the ELF object */
-			crcp = sym_get_data(info, sym);
-			crc = TO_NATIVE(*crcp);
-		}
-		sym_update_crc(symname + strlen("__crc_"), mod, crc,
-				export);
-	}
-
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
 		if (strstarts(symname, "__gnu_lto_")) {
@@ -746,11 +753,6 @@  static void handle_symbol(struct module *mod, struct elf_info *info,
 		}
 #endif
 
-		if (is_crc) {
-			const char *e = is_vmlinux(mod->name) ?"":".ko";
-			warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n",
-			     symname + strlen("__crc_"), mod->name, e);
-		}
 		mod->unres = alloc_symbol(symname,
 					  ELF_ST_BIND(sym->st_info) == STB_WEAK,
 					  mod->unres);
@@ -2063,6 +2065,10 @@  static void read_symbols(const char *modname)
 			sym_update_namespace(symname + strlen("__kstrtabns_"),
 					     namespace_from_kstrtabns(&info,
 								      sym));
+
+		if (strstarts(symname, "__crc_"))
+			handle_modversion(mod, &info, sym,
+					  symname + strlen("__crc_"));
 	}
 
 	// check for static EXPORT_SYMBOL_* functions && global vars
@@ -2476,7 +2482,7 @@  static void read_dump(const char *fname, unsigned int kernel)
 		s->kernel    = kernel;
 		s->preloaded = 1;
 		s->is_static = 0;
-		sym_update_crc(symname, mod, crc, export_no(export));
+		sym_set_crc(symname, mod, crc);
 		sym_update_namespace(symname, namespace);
 	}
 	release_file(file, size);