On Mon, 22 Aug 2016 20:47:58 +1000
Nicholas Piggin [off-list ref] wrote:

On Fri, 19 Aug 2016 20:44:55 +1000
Nicholas Piggin [off-list ref] wrote:

quoted

On Fri, 19 Aug 2016 10:37:00 +0200
Michal Marek [off-list ref] wrote:
  

quoted

On 2016-08-19 07:09, Stephen Rothwell wrote:    

[snip]

quoted

quoted

quoted

I may be missing something, but genksyms generates the crc's off the
preprocessed C source code and we don't have any for the asm files ...      

Of course you are right. Which means that we are losing type information
for these exports for CONFIG_MODVERSIONS purposes. I guess it's
acceptable, since the asm functions are pretty basic and their
signatures do not change.    

I don't completely agree. It would be nice to have the functionality
still there.

What happens if you just run cmd_modversions on the as rule? It relies on
!defined(__ASSEMBLY__), but we're feeding the result to genksyms, not as.
It would require the header be included in the .S file and be protected for
asm builds.  

This seems like it *could* be made to work, but there's a few problems.

- .h files are not made for C consumption. Matter of manually adding the
ifdef guards, which isn't terrible.

- .S files do not all include their .h where the C declaration is. Also
will cause some churn but doable and maybe not completely unreasonable.

- genksyms parser barfs when it hits the assembly of the .S file. Best
way to fix that seems just send the #include and EXPORT_SYMBOL lines
from the .S to the preprocessor. That's a bit of a rabbit hole too, with
some .S files being included, etc.

I'm not sure what to do here. If nobody cares and we lose CRCs for .S
exports, then okay we can whitelist those relocs easily. If we don't want
to lose the functionality, the above might work but it's a bit intrusive
an is going to require another cycle of prep patches to go through arch
code first.

Or suggestions for alternative approach?

Here is a quick patch that I think should catch missing CRCs in
architecture independent way. If we merge something like this, we
can whitelist the symbols in arch/powerpc so people get steered to
the right place.

Powerpc seems to be the only one really catching this, and it's
only as a side effect of a test run for CONFIG_RELOCATABLE kernels,
which means version failures probably slipped through other archs.

I'll clean it up, do some more testing, and submit it unless
anybody dislikes it or has a better way to do it.

Thanks,
Nick

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4b8ffd3..1efc454 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c

@@ -609,6 +609,7 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 {
 	unsigned int crc;
 	enum export export;
+	int is_crc = 0;
 
 	if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
 	    strncmp(symname, "__ksymtab", 9) == 0)

@@ -618,6 +619,7 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 
 	/* CRC'd symbol */
 	if (strncmp(symname, CRC_PFX, strlen(CRC_PFX)) == 0) {
+		is_crc = 1;
 		crc = (unsigned int) sym->st_value;
 		sym_update_crc(symname + strlen(CRC_PFX), mod, crc,
 				export);

@@ -663,6 +665,10 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 		else
 			symname++;
 #endif
+		if (is_crc && !mod->is_dot_o) {
+			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_PFX), mod->name, e);
+		}
 		mod->unres = alloc_symbol(symname,
 					  ELF_ST_BIND(sym->st_info) == STB_WEAK,
 					  mod->unres);