[PATCH 1/3] MODSIGN: Fix comparison erros in scripts/sign-file

[PATCH 1/3] MODSIGN: Fix comparison erros in scripts/sign-file

[Michal Marek]

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 scripts/sign-file |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/sign-file b/scripts/sign-file
index 87ca59d..974a20b 100755
--- a/scripts/sign-file
+++ b/scripts/sign-file
@@ -156,12 +156,12 @@ sub asn1_extract($$@)
 
 	if ($l == 0x1) {
 	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1));
-	} elsif ($l = 0x2) {
+	} elsif ($l == 0x2) {
 	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0], 2));
-	} elsif ($l = 0x3) {
+	} elsif ($l == 0x3) {
 	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1)) << 16;
 	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0] + 1, 2));
-	} elsif ($l = 0x4) {
+	} elsif ($l == 0x4) {
 	    $len = unpack("N", substr(${$cursor->[2]}, $cursor->[0], 4));
 	} else {
 	    die $x509, ": ", $cursor->[0], ": ASN.1 element too long (", $l, ")\n";
-- 
1.7.10.4

[PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Michal Marek]

From: Takashi Iwai <tiwai@suse.de>

Using the asm .incbin statement in C sources breaks any gcc wrapper which
assumes that preprocessed C source is self-contained. Use a separate .S
file to include the siging key and certificate.

Tested-by: Michal Marek <mmarek@suse.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 kernel/Makefile              |    4 ++--
 kernel/modsign_certificate.S |    9 +++++++++
 kernel/modsign_pubkey.c      |    6 ------
 3 files changed, 11 insertions(+), 8 deletions(-)
 create mode 100644 kernel/modsign_certificate.S

diff --git a/kernel/Makefile b/kernel/Makefile
index 86e3285..0bd9d43 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o
+obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
@@ -139,7 +139,7 @@ ifeq ($(CONFIG_MODULE_SIG),y)
 extra_certificates:
 	touch $@
 
-kernel/modsign_pubkey.o: signing_key.x509 extra_certificates
+kernel/modsign_certificate.o: signing_key.x509 extra_certificates
 
 ###############################################################################
 #
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
new file mode 100644
index 0000000..6bde029
--- /dev/null
+++ b/kernel/modsign_certificate.S
@@ -0,0 +1,9 @@
+#include <linux/linkage.h>
+
+	.section ".init.data","aw"
+
+GLOBAL(modsign_certificate_list)
+	.incbin "signing_key.x509"
+	.incbin "extra_certificates"
+END(modsign_certificate_list)
+GLOBAL(modsign_certificate_list_end)
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 4646eb2..045504f 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -20,12 +20,6 @@ struct key *modsign_keyring;
 
 extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
-asm(".section .init.data,\"aw\"\n"
-    "modsign_certificate_list:\n"
-    ".incbin \"signing_key.x509\"\n"
-    ".incbin \"extra_certificates\"\n"
-    "modsign_certificate_list_end:"
-    );
 
 /*
  * We need to make sure ccache doesn't cache the .o file as it doesn't notice
-- 
1.7.10.4

[PATCH 3/3] MODSIGN: Drop ccache hack

[Michal Marek]

The __TIME__ macro is not needed anymore, because the pubkey is included
in a separate .S file.

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 kernel/modsign_pubkey.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 045504f..df27eca 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -22,12 +22,6 @@ extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
 
 /*
- * We need to make sure ccache doesn't cache the .o file as it doesn't notice
- * if modsign.pub changes.
- */
-static __initdata const char annoy_ccache[] = __TIME__ "foo";
-
-/*
  * Load the compiled-in keys
  */
 static __init int module_verify_init(void)
-- 
1.7.10.4

Re: [PATCH 1/3] MODSIGN: Fix comparison erros in scripts/sign-file

[David Howells]

Michal Marek <mmarek@suse.cz> wrote:

> Signed-off-by: Michal Marek <mmarek@suse.cz>

If only perl gave you warnings for this as gcc does...

Acked-by: David Howells <dhowells@redhat.com>

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[David Howells]

Michal Marek <mmarek@suse.cz> wrote:

> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> assumes that preprocessed C source is self-contained. Use a separate .S
> file to include the siging key and certificate.

I was trying to avoid that as .S files generally don't crop up in generic
code and the format of the assembly varies with the arch.  However, you don't
seem to have anything that should cause a problem - so:

Acked-by: David Howells <dhowells@redhat.com>

Re: [PATCH 3/3] MODSIGN: Drop ccache hack

[David Howells]

Michal Marek <mmarek@suse.cz> wrote:

> The __TIME__ macro is not needed anymore, because the pubkey is included
> in a separate .S file.

Acked-by: David Howells <dhowells@redhat.com>

Re: [PATCH 1/3] MODSIGN: Fix comparison erros in scripts/sign-file

[Rusty Russell]

Michal Marek <mmarek@suse.cz> writes:

> Signed-off-by: Michal Marek <mmarek@suse.cz>
> ---
>  scripts/sign-file |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/sign-file b/scripts/sign-file
> index 87ca59d..974a20b 100755
> --- a/scripts/sign-file
> +++ b/scripts/sign-file
> @@ -156,12 +156,12 @@ sub asn1_extract($$@)
>  
>  	if ($l == 0x1) {
>  	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1));
> -	} elsif ($l = 0x2) {
> +	} elsif ($l == 0x2) {
>  	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0], 2));
> -	} elsif ($l = 0x3) {
> +	} elsif ($l == 0x3) {
>  	    $len = unpack("C", substr(${$cursor->[2]}, $cursor->[0], 1)) << 16;
>  	    $len = unpack("n", substr(${$cursor->[2]}, $cursor->[0] + 1, 2));
> -	} elsif ($l = 0x4) {
> +	} elsif ($l == 0x4) {
>  	    $len = unpack("N", substr(${$cursor->[2]}, $cursor->[0], 4));
>  	} else {
>  	    die $x509, ": ", $cursor->[0], ": ASN.1 element too long (", $l, ")\n";

This was already applied to Linus' tree as 

commit 916492b1e1a186260951831c53a53d8a448dc026
Author: Chun-Yi Lee <jlee@suse.com>
Date:   Wed Nov 21 11:26:09 2012 +0000

Thanks,
Rusty.

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Rusty Russell]

David Howells <dhowells@redhat.com> writes:

> Michal Marek <mmarek@suse.cz> wrote:
>
>> Using the asm .incbin statement in C sources breaks any gcc wrapper which
>> assumes that preprocessed C source is self-contained. Use a separate .S
>> file to include the siging key and certificate.
>
> I was trying to avoid that as .S files generally don't crop up in generic
> code and the format of the assembly varies with the arch.  However, you don't
> seem to have anything that should cause a problem - so:
>
> Acked-by: David Howells <dhowells@redhat.com>

GLOBAL() is defined in x86 only, AFAICT.

Plus, we now have a patch from James (CC:d) to prepend
CONFIG_SYMBOL_PREFIX to these as requird.

I think this will have to be done more carefully...

Thanks,
Rusty.

From: Takashi Iwai <tiwai@suse.de>

Using the asm .incbin statement in C sources breaks any gcc wrapper which
assumes that preprocessed C source is self-contained. Use a separate .S
file to include the siging key and certificate.

Tested-by: Michal Marek <mmarek@suse.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 kernel/Makefile              |    4 ++--
 kernel/modsign_certificate.S |    9 +++++++++
 kernel/modsign_pubkey.c      |    6 ------
 3 files changed, 11 insertions(+), 8 deletions(-)
 create mode 100644 kernel/modsign_certificate.S

diff --git a/kernel/Makefile b/kernel/Makefile
index 86e3285..0bd9d43 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o
+obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
@@ -139,7 +139,7 @@ ifeq ($(CONFIG_MODULE_SIG),y)
 extra_certificates:
 	touch $@
 
-kernel/modsign_pubkey.o: signing_key.x509 extra_certificates
+kernel/modsign_certificate.o: signing_key.x509 extra_certificates
 
 ###############################################################################
 #
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
new file mode 100644
index 0000000..6bde029
--- /dev/null
+++ b/kernel/modsign_certificate.S
@@ -0,0 +1,9 @@
+#include <linux/linkage.h>
+
+	.section ".init.data","aw"
+
+GLOBAL(modsign_certificate_list)
+	.incbin "signing_key.x509"
+	.incbin "extra_certificates"
+END(modsign_certificate_list)
+GLOBAL(modsign_certificate_list_end)
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 4646eb2..045504f 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -20,12 +20,6 @@ struct key *modsign_keyring;
 
 extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
-asm(".section .init.data,\"aw\"\n"
-    "modsign_certificate_list:\n"
-    ".incbin \"signing_key.x509\"\n"
-    ".incbin \"extra_certificates\"\n"
-    "modsign_certificate_list_end:"
-    );
 
 /*
  * We need to make sure ccache doesn't cache the .o file as it doesn't notice
-- 
1.7.10.4

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Takashi Iwai]

At Wed, 05 Dec 2012 10:28:48 +1030,
Rusty Russell wrote:
> 
> David Howells <dhowells@redhat.com> writes:
> 
> > Michal Marek <mmarek@suse.cz> wrote:
> >
> >> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> >> assumes that preprocessed C source is self-contained. Use a separate .S
> >> file to include the siging key and certificate.
> >
> > I was trying to avoid that as .S files generally don't crop up in generic
> > code and the format of the assembly varies with the arch.  However, you don't
> > seem to have anything that should cause a problem - so:
> >
> > Acked-by: David Howells <dhowells@redhat.com>
> 
> GLOBAL() is defined in x86 only, AFAICT.
> 
> Plus, we now have a patch from James (CC:d) to prepend
> CONFIG_SYMBOL_PREFIX to these as requird.

Yes, I noticed both things yesterday.

> 
> I think this will have to be done more carefully...

How about the revised patch below?


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] MODSIGN: Avoid using .incbin in C source

Using the asm .incbin statement in C sources breaks any gcc wrapper which
assumes that preprocessed C source is self-contained. Use a separate .S
file to include the siging key and certificate.

Tested-by: Michal Marek <mmarek@suse.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 kernel/Makefile              |  4 ++--
 kernel/modsign_certificate.S | 18 ++++++++++++++++++
 kernel/modsign_pubkey.c      |  6 ------
 3 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 kernel/modsign_certificate.S

diff --git a/kernel/Makefile b/kernel/Makefile
index 86e3285..0bd9d43 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o
+obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
@@ -139,7 +139,7 @@ ifeq ($(CONFIG_MODULE_SIG),y)
 extra_certificates:
 	touch $@
 
-kernel/modsign_pubkey.o: signing_key.x509 extra_certificates
+kernel/modsign_certificate.o: signing_key.x509 extra_certificates
 
 ###############################################################################
 #
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
new file mode 100644
index 0000000..695d4e3
--- /dev/null
+++ b/kernel/modsign_certificate.S
@@ -0,0 +1,18 @@
+#ifndef SYMBOL_PREFIX
+#define ASM_SYMBOL(sym) sym
+#else
+#define PASTE2(x,y) x##y
+#define PASTE(x,y) PASTE2(x,y)
+#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
+#endif
+
+#define GLOBAL(name)	\
+	.globl ASM_SYMBOL(name);	\
+	ASM_SYMBOL(name):
+
+	.section ".init.data","aw"
+
+GLOBAL(modsign_certificate_list)
+	.incbin "signing_key.x509"
+	.incbin "extra_certificates"
+GLOBAL(modsign_certificate_list_end)
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 767e559..045504f 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -20,12 +20,6 @@ struct key *modsign_keyring;
 
 extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
-asm(".section .init.data,\"aw\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list:\n"
-    ".incbin \"signing_key.x509\"\n"
-    ".incbin \"extra_certificates\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list_end:"
-    );
 
 /*
  * We need to make sure ccache doesn't cache the .o file as it doesn't notice
-- 
1.8.0.1

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Takashi Iwai]

At Tue, 04 Dec 2012 18:17:28 +0000,
David Howells wrote:
> 
> Michal Marek <mmarek@suse.cz> wrote:
> 
> > Using the asm .incbin statement in C sources breaks any gcc wrapper which
> > assumes that preprocessed C source is self-contained. Use a separate .S
> > file to include the siging key and certificate.
> 
> I was trying to avoid that as .S files generally don't crop up in generic
> code and the format of the assembly varies with the arch.  However, you don't
> seem to have anything that should cause a problem

Well, we hit actually the issue...  Please check the revised patch again.


thanks,

Takashi

> - so:
> 
> Acked-by: David Howells <dhowells@redhat.com>
> 

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Michal Marek]

On Wed, Dec 05, 2012 at 08:39:11AM +0100, Takashi Iwai wrote:
> At Wed, 05 Dec 2012 10:28:48 +1030,
> Rusty Russell wrote:
> > 
> > David Howells <dhowells@redhat.com> writes:
> > 
> > > Michal Marek <mmarek@suse.cz> wrote:
> > >
> > >> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> > >> assumes that preprocessed C source is self-contained. Use a separate .S
> > >> file to include the siging key and certificate.
> > >
> > > I was trying to avoid that as .S files generally don't crop up in generic
> > > code and the format of the assembly varies with the arch.  However, you don't
> > > seem to have anything that should cause a problem - so:
> > >
> > > Acked-by: David Howells <dhowells@redhat.com>
> > 
> > GLOBAL() is defined in x86 only, AFAICT.

Sorry for that, the Tested-by actually meant Tested-on-x86_64-by :/


> How about the revised patch below?
[...]
> diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
> new file mode 100644
> index 0000000..695d4e3
> --- /dev/null
> +++ b/kernel/modsign_certificate.S
> @@ -0,0 +1,18 @@
> +#ifndef SYMBOL_PREFIX
> +#define ASM_SYMBOL(sym) sym
> +#else
> +#define PASTE2(x,y) x##y
> +#define PASTE(x,y) PASTE2(x,y)
> +#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
> +#endif


It looks good, but looking at your patch, I just noticed that we have two
versions of the SYMBOL_PREFIX macro in the kernel now:

scripts/Makefile.lib has had since some time

ifdef CONFIG_SYMBOL_PREFIX
_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
_cpp_flags += $(_sym_flags)
_a_flags += $(_sym_flags)
endif

while include/linux/kernel.h now has

/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
#ifdef CONFIG_SYMBOL_PREFIX
#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
#else
#define SYMBOL_PREFIX ""
#endif

So depending on whether you include <linux/kernel.h> or not,
SYMBOL_PREFIX expands to either a string or a token. James, Rusty, how
about the patch below? If Takashi's patch is applied first, then
obviously the modsign_pubkey.c part should be dropped and the ASM_SYMBOL
definition in modsign_certificate.S can be simplified instead.

Michal

>From df545c46fb89c085387f4d98c7a4fb06a7f678a9 Mon Sep 17 00:00:00 2001
From: Michal Marek <mmarek@suse.cz>
Date: Wed, 5 Dec 2012 10:03:15 +0100
Subject: [PATCH] linux/kernel.h: Avoid conflicting SYMBOL_PREFIX definition

scripts/Makefile.lib already defines SYMBOL_PREFIX to the token _ if
CONFIG_SYMBOL_PREFIX is defined. Drop the string definition in
linux/kernel.h and change scripts/Makefile.lib to define SYMBOL_PREFIX
on all architectures.

Signed-off-by: Michal Marek <mmarek@suse.cz>
---
 include/asm-generic/vmlinux.lds.h |    4 ----
 include/linux/kernel.h            |    7 -------
 kernel/modsign_pubkey.c           |    5 +++--
 scripts/Makefile.lib              |    5 ++---
 4 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index d1ea7ce..7756a0c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,13 +52,9 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
-#define VMLINUX_SYMBOL(sym) sym
-#else
 #define PASTE2(x,y) x##y
 #define PASTE(x,y) PASTE2(x,y)
 #define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
 #define ALIGN_FUNCTION()  . = ALIGN(8)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d140e8f..9a6bccb 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -717,13 +717,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 767e559..93ce84b 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -9,6 +9,7 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#include <linux/stringify.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/cred.h>
@@ -21,10 +22,10 @@ struct key *modsign_keyring;
 extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
 asm(".section .init.data,\"aw\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list:\n"
+    __stringify(SYMBOL_PREFIX) "modsign_certificate_list:\n"
     ".incbin \"signing_key.x509\"\n"
     ".incbin \"extra_certificates\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list_end:"
+    __stringify(SYMBOL_PREFIX) "modsign_certificate_list_end:"
     );
 
 /*
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index bdf42fd..1138e77 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,11 +119,10 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
 _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
+_c_flags += $(_sym_flags)
 _a_flags += $(_sym_flags)
-endif
+_cpp_flags += $(_sym_flags)
 
 
 # If building the kernel in a separate objtree expand all occurrences
-- 
1.7.10.4

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Takashi Iwai]

At Wed, 5 Dec 2012 10:50:57 +0100,
Michal Marek wrote:
> 
> On Wed, Dec 05, 2012 at 08:39:11AM +0100, Takashi Iwai wrote:
> > At Wed, 05 Dec 2012 10:28:48 +1030,
> > Rusty Russell wrote:
> > > 
> > > David Howells <dhowells@redhat.com> writes:
> > > 
> > > > Michal Marek <mmarek@suse.cz> wrote:
> > > >
> > > >> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> > > >> assumes that preprocessed C source is self-contained. Use a separate .S
> > > >> file to include the siging key and certificate.
> > > >
> > > > I was trying to avoid that as .S files generally don't crop up in generic
> > > > code and the format of the assembly varies with the arch.  However, you don't
> > > > seem to have anything that should cause a problem - so:
> > > >
> > > > Acked-by: David Howells <dhowells@redhat.com>
> > > 
> > > GLOBAL() is defined in x86 only, AFAICT.
> 
> Sorry for that, the Tested-by actually meant Tested-on-x86_64-by :/
> 
> 
> > How about the revised patch below?
> [...]
> > diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
> > new file mode 100644
> > index 0000000..695d4e3
> > --- /dev/null
> > +++ b/kernel/modsign_certificate.S
> > @@ -0,0 +1,18 @@
> > +#ifndef SYMBOL_PREFIX
> > +#define ASM_SYMBOL(sym) sym
> > +#else
> > +#define PASTE2(x,y) x##y
> > +#define PASTE(x,y) PASTE2(x,y)
> > +#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
> > +#endif
> 
> 
> It looks good, but looking at your patch, I just noticed that we have two
> versions of the SYMBOL_PREFIX macro in the kernel now:
> 
> scripts/Makefile.lib has had since some time
> 
> ifdef CONFIG_SYMBOL_PREFIX
> _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> _cpp_flags += $(_sym_flags)
> _a_flags += $(_sym_flags)
> endif
> 
> while include/linux/kernel.h now has
> 
> /* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
> #ifdef CONFIG_SYMBOL_PREFIX
> #define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> #else
> #define SYMBOL_PREFIX ""
> #endif

Urgh.  Strange that I didn't notice the error when I tested with the
patch (with artificial CONFIG_SYMBOL_PREFIX set in
arch/x86/Kconfig)...


> So depending on whether you include <linux/kernel.h> or not,
> SYMBOL_PREFIX expands to either a string or a token. James, Rusty, how
> about the patch below? If Takashi's patch is applied first, then
> obviously the modsign_pubkey.c part should be dropped and the ASM_SYMBOL
> definition in modsign_certificate.S can be simplified instead.
> 
> Michal
> 
> >From df545c46fb89c085387f4d98c7a4fb06a7f678a9 Mon Sep 17 00:00:00 2001
> From: Michal Marek <mmarek@suse.cz>
> Date: Wed, 5 Dec 2012 10:03:15 +0100
> Subject: [PATCH] linux/kernel.h: Avoid conflicting SYMBOL_PREFIX definition
> 
> scripts/Makefile.lib already defines SYMBOL_PREFIX to the token _ if
> CONFIG_SYMBOL_PREFIX is defined. Drop the string definition in
> linux/kernel.h and change scripts/Makefile.lib to define SYMBOL_PREFIX
> on all architectures.
> 
> Signed-off-by: Michal Marek <mmarek@suse.cz>

Looks like a good cleanup.
Reviewed-by: Takashi Iwai <tiwai@suse.de>


Takashi

> ---
>  include/asm-generic/vmlinux.lds.h |    4 ----
>  include/linux/kernel.h            |    7 -------
>  kernel/modsign_pubkey.c           |    5 +++--
>  scripts/Makefile.lib              |    5 ++---
>  4 files changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index d1ea7ce..7756a0c 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -52,13 +52,9 @@
>  #define LOAD_OFFSET 0
>  #endif
>  
> -#ifndef SYMBOL_PREFIX
> -#define VMLINUX_SYMBOL(sym) sym
> -#else
>  #define PASTE2(x,y) x##y
>  #define PASTE(x,y) PASTE2(x,y)
>  #define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
> -#endif
>  
>  /* Align . to a 8 byte boundary equals to maximum function alignment. */
>  #define ALIGN_FUNCTION()  . = ALIGN(8)
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index d140e8f..9a6bccb 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -717,13 +717,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>  /* Trap pasters of __FUNCTION__ at compile-time */
>  #define __FUNCTION__ (__func__)
>  
> -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
> -#ifdef CONFIG_SYMBOL_PREFIX
> -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> -#else
> -#define SYMBOL_PREFIX ""
> -#endif
> -
>  /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
>  # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
> index 767e559..93ce84b 100644
> --- a/kernel/modsign_pubkey.c
> +++ b/kernel/modsign_pubkey.c
> @@ -9,6 +9,7 @@
>   * 2 of the Licence, or (at your option) any later version.
>   */
>  
> +#include <linux/stringify.h>
>  #include <linux/kernel.h>
>  #include <linux/sched.h>
>  #include <linux/cred.h>
> @@ -21,10 +22,10 @@ struct key *modsign_keyring;
>  extern __initdata const u8 modsign_certificate_list[];
>  extern __initdata const u8 modsign_certificate_list_end[];
>  asm(".section .init.data,\"aw\"\n"
> -    SYMBOL_PREFIX "modsign_certificate_list:\n"
> +    __stringify(SYMBOL_PREFIX) "modsign_certificate_list:\n"
>      ".incbin \"signing_key.x509\"\n"
>      ".incbin \"extra_certificates\"\n"
> -    SYMBOL_PREFIX "modsign_certificate_list_end:"
> +    __stringify(SYMBOL_PREFIX) "modsign_certificate_list_end:"
>      );
>  
>  /*
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index bdf42fd..1138e77 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -119,11 +119,10 @@ _c_flags += $(if $(patsubst n%,, \
>  		$(CFLAGS_GCOV))
>  endif
>  
> -ifdef CONFIG_SYMBOL_PREFIX
>  _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> -_cpp_flags += $(_sym_flags)
> +_c_flags += $(_sym_flags)
>  _a_flags += $(_sym_flags)
> -endif
> +_cpp_flags += $(_sym_flags)
>  
>  
>  # If building the kernel in a separate objtree expand all occurrences
> -- 
> 1.7.10.4
> 
> 

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[David Howells]

Takashi Iwai <tiwai@suse.de> wrote:

> +#ifndef SYMBOL_PREFIX

This comes via the command line?

David

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[James Hogan]

On 05/12/12 09:50, Michal Marek wrote:
>> How about the revised patch below?
> [...]
>> diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
>> new file mode 100644
>> index 0000000..695d4e3
>> --- /dev/null
>> +++ b/kernel/modsign_certificate.S
>> @@ -0,0 +1,18 @@
>> +#ifndef SYMBOL_PREFIX
>> +#define ASM_SYMBOL(sym) sym
>> +#else
>> +#define PASTE2(x,y) x##y
>> +#define PASTE(x,y) PASTE2(x,y)
>> +#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
>> +#endif
> 
> 
> It looks good, but looking at your patch, I just noticed that we have two
> versions of the SYMBOL_PREFIX macro in the kernel now:
> 
> scripts/Makefile.lib has had since some time
> 
> ifdef CONFIG_SYMBOL_PREFIX
> _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> _cpp_flags += $(_sym_flags)
> _a_flags += $(_sym_flags)
> endif
> 
> while include/linux/kernel.h now has
> 
> /* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
> #ifdef CONFIG_SYMBOL_PREFIX
> #define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> #else
> #define SYMBOL_PREFIX ""
> #endif
> 
> So depending on whether you include <linux/kernel.h> or not,
> SYMBOL_PREFIX expands to either a string or a token. James, Rusty, how
> about the patch below? If Takashi's patch is applied first, then
> obviously the modsign_pubkey.c part should be dropped and the ASM_SYMBOL
> definition in modsign_certificate.S can be simplified instead.
> 
> Michal
> 
> From df545c46fb89c085387f4d98c7a4fb06a7f678a9 Mon Sep 17 00:00:00 2001
> From: Michal Marek <mmarek@suse.cz>
> Date: Wed, 5 Dec 2012 10:03:15 +0100
> Subject: [PATCH] linux/kernel.h: Avoid conflicting SYMBOL_PREFIX definition
> 
> scripts/Makefile.lib already defines SYMBOL_PREFIX to the token _ if
> CONFIG_SYMBOL_PREFIX is defined. Drop the string definition in
> linux/kernel.h and change scripts/Makefile.lib to define SYMBOL_PREFIX
> on all architectures.

Hi Michal,

Yeh, that looks good to me, and I like it just being automatically
defined rather than having to include linux/kernel.h (potentially from
low level architecture headers too).

However I think it's unfortunate having to stringify from C as it's
pretty much always required to be in string form when used from a C
file, usually in an asm block. Any objection to defining SYMBOL_PREFIX
with the quotes around it for _c_flags only? E.g. see below

Cheers
James

> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index bdf42fd..1138e77 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -119,11 +119,10 @@ _c_flags += $(if $(patsubst n%,, \
>  		$(CFLAGS_GCOV))
>  endif
>  
> -ifdef CONFIG_SYMBOL_PREFIX
>  _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> -_cpp_flags += $(_sym_flags)
> +_c_flags += $(_sym_flags)
>  _a_flags += $(_sym_flags)
> -endif
> +_cpp_flags += $(_sym_flags)
>  
>  
>  # If building the kernel in a separate objtree expand all occurrences
> 

How about this instead (i.e. no changes to kernel/modsign_pubkey.c)?

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0be6f11..a76967e 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,11 +119,11 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
 _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
+_c_sym_flags = -DSYMBOL_PREFIX=\"$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))\"
+_c_flags += $(_c_sym_flags)
 _a_flags += $(_sym_flags)
-endif
+_cpp_flags += $(_sym_flags)
 
 
 # If building the kernel in a separate objtree expand all occurrences

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Michal Marek]

Dne 5.12.2012 11:30, David Howells napsal(a):
> Takashi Iwai <tiwai@suse.de> wrote:
> 
>> +#ifndef SYMBOL_PREFIX
> 
> This comes via the command line?

Yes, see scripts/Makefile.lib:

ifdef CONFIG_SYMBOL_PREFIX
_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
_cpp_flags += $(_sym_flags)
_a_flags += $(_sym_flags)
endif

Michal

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Michal Marek]

Dne 5.12.2012 11:30, James Hogan napsal(a):
> However I think it's unfortunate having to stringify from C as it's
> pretty much always required to be in string form when used from a C
> file, usually in an asm block. Any objection to defining SYMBOL_PREFIX
> with the quotes around it for _c_flags only? E.g. see below


After Takashi's patch is applied, there will be no user of the
SYMBOL_PREFIX in C source. But it would probably be more convenient for
potential new users.

> +_c_sym_flags = -DSYMBOL_PREFIX=\"$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))\"

Maybe simply

-DDSYMBOL_PREFIX='$(or $(CONFIG_SYMBOL_PREFIX),"")'

? The single quotes are needed either way, to prevent the shell eating
the double quotes.

Michal

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[James Hogan]

On 05/12/12 11:05, Michal Marek wrote:
> Dne 5.12.2012 11:30, James Hogan napsal(a):
>> However I think it's unfortunate having to stringify from C as it's
>> pretty much always required to be in string form when used from a C
>> file, usually in an asm block. Any objection to defining SYMBOL_PREFIX
>> with the quotes around it for _c_flags only? E.g. see below
> 
> 
> After Takashi's patch is applied, there will be no user of the
> SYMBOL_PREFIX in C source. But it would probably be more convenient for
> potential new users.

Yes indeed (so the C related bits could indeed be dropped for now). I'd
likely change this patch to use it though:
http://thread.gmane.org/gmane.linux.kernel.cross-arch/15586/focus=15593

> 
>> +_c_sym_flags = -DSYMBOL_PREFIX=\"$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))\"
> 
> Maybe simply
> 
> -DDSYMBOL_PREFIX='$(or $(CONFIG_SYMBOL_PREFIX),"")'
> 
> ? The single quotes are needed either way, to prevent the shell eating
> the double quotes.

Yep, works for me (makefiles aren't one of my strengths!)

Cheers
James

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[David Howells]

Takashi Iwai <tiwai@suse.de> wrote:

> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH v2] MODSIGN: Avoid using .incbin in C source
> 
> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> assumes that preprocessed C source is self-contained. Use a separate .S
> file to include the siging key and certificate.
> 
> Tested-by: Michal Marek <mmarek@suse.cz>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Acked-by: David Howells <dhowells@redhat.com>

> +#define GLOBAL(name)	\
> +	.globl ASM_SYMBOL(name);	\
> +	ASM_SYMBOL(name):

This should perhaps be moved into a global header.

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[Rusty Russell]

Michal Marek <mmarek@suse.cz> writes:
> It looks good, but looking at your patch, I just noticed that we have two
> versions of the SYMBOL_PREFIX macro in the kernel now:
>
> scripts/Makefile.lib has had since some time
>
> ifdef CONFIG_SYMBOL_PREFIX
> _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> _cpp_flags += $(_sym_flags)
> _a_flags += $(_sym_flags)
> endif
>
> while include/linux/kernel.h now has
>
> /* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
> #ifdef CONFIG_SYMBOL_PREFIX
> #define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> #else
> #define SYMBOL_PREFIX ""
> #endif

What a mess!  AFAICT there's one place which a non-string SYMBOL_PREFIX:
        include/asm-generic/vmlinux.lds.h

So we do need it, and the only sane way seems to be via the cmdline and
the shell usefully stripping quotes as we do.

So I just took Takashi's do-it-all-in-asm fix, and removed this
re-definition (we have the same thing, BTW, called
MODULE_SYMBOL_PREFIX).

I added a comment about the cmdline SYMBOL_PREFIX, too.

Thanks,
Rusty.

From: Takashi Iwai <tiwai@suse.de>
Subject: MODSIGN: Avoid using .incbin in C source

Using the asm .incbin statement in C sources breaks any gcc wrapper which
assumes that preprocessed C source is self-contained. Use a separate .S
file to include the siging key and certificate.

[ This means we no longer need SYMBOL_PREFIX which is defined in kernel.h
  from cbdbf2abb7844548a7d7a6a2ae7af6b6fbcea401, so I removed it -- RR ]

Tested-by: Michal Marek <mmarek@suse.cz>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7d8dfc7..a123b13 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -701,13 +701,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define COMPACTION_BUILD 0
 #endif
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/kernel/Makefile b/kernel/Makefile
index 86e3285..0bd9d43 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -54,7 +54,7 @@ obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
 obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
 obj-$(CONFIG_UID16) += uid16.o
 obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o
+obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
 obj-$(CONFIG_KALLSYMS) += kallsyms.o
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
@@ -139,7 +139,7 @@ ifeq ($(CONFIG_MODULE_SIG),y)
 extra_certificates:
 	touch $@
 
-kernel/modsign_pubkey.o: signing_key.x509 extra_certificates
+kernel/modsign_certificate.o: signing_key.x509 extra_certificates
 
 ###############################################################################
 #
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
new file mode 100644
index 0000000..246b4c6
--- /dev/null
+++ b/kernel/modsign_certificate.S
@@ -0,0 +1,19 @@
+/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
+#ifndef SYMBOL_PREFIX
+#define ASM_SYMBOL(sym) sym
+#else
+#define PASTE2(x,y) x##y
+#define PASTE(x,y) PASTE2(x,y)
+#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
+#endif
+
+#define GLOBAL(name)	\
+	.globl ASM_SYMBOL(name);	\
+	ASM_SYMBOL(name):
+
+	.section ".init.data","aw"
+
+GLOBAL(modsign_certificate_list)
+	.incbin "signing_key.x509"
+	.incbin "extra_certificates"
+GLOBAL(modsign_certificate_list_end)
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
index 767e559..045504f 100644
--- a/kernel/modsign_pubkey.c
+++ b/kernel/modsign_pubkey.c
@@ -20,12 +20,6 @@ struct key *modsign_keyring;
 
 extern __initdata const u8 modsign_certificate_list[];
 extern __initdata const u8 modsign_certificate_list_end[];
-asm(".section .init.data,\"aw\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list:\n"
-    ".incbin \"signing_key.x509\"\n"
-    ".incbin \"extra_certificates\"\n"
-    SYMBOL_PREFIX "modsign_certificate_list_end:"
-    );
 
 /*
  * We need to make sure ccache doesn't cache the .o file as it doesn't notice

Re: [PATCH 2/3] MODSIGN: Avoid using .incbin in C source

[James Hogan]

On 07/12/12 04:40, Rusty Russell wrote:
> Michal Marek <mmarek@suse.cz> writes:
>> It looks good, but looking at your patch, I just noticed that we have two
>> versions of the SYMBOL_PREFIX macro in the kernel now:
>>
>> scripts/Makefile.lib has had since some time
>>
>> ifdef CONFIG_SYMBOL_PREFIX
>> _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
>> _cpp_flags += $(_sym_flags)
>> _a_flags += $(_sym_flags)
>> endif
>>
>> while include/linux/kernel.h now has
>>
>> /* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
>> #ifdef CONFIG_SYMBOL_PREFIX
>> #define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
>> #else
>> #define SYMBOL_PREFIX ""
>> #endif
> 
> What a mess!  AFAICT there's one place which a non-string SYMBOL_PREFIX:
>         include/asm-generic/vmlinux.lds.h
> 
> So we do need it, and the only sane way seems to be via the cmdline and
> the shell usefully stripping quotes as we do.
> 
> So I just took Takashi's do-it-all-in-asm fix, and removed this
> re-definition (we have the same thing, BTW, called
> MODULE_SYMBOL_PREFIX).
> 
> I added a comment about the cmdline SYMBOL_PREFIX, too.
> 
> Thanks,
> Rusty.
> 
> From: Takashi Iwai <tiwai@suse.de>
> Subject: MODSIGN: Avoid using .incbin in C source
> 
> Using the asm .incbin statement in C sources breaks any gcc wrapper which
> assumes that preprocessed C source is self-contained. Use a separate .S
> file to include the siging key and certificate.
> 
> [ This means we no longer need SYMBOL_PREFIX which is defined in kernel.h
>   from cbdbf2abb7844548a7d7a6a2ae7af6b6fbcea401, so I removed it -- RR ]
> 
> Tested-by: Michal Marek <mmarek@suse.cz>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Looks good to me.
Acked-by: James Hogan <james.hogan@imgtec.com>

Thanks
James

> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 7d8dfc7..a123b13 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -701,13 +701,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>  #define COMPACTION_BUILD 0
>  #endif
>  
> -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
> -#ifdef CONFIG_SYMBOL_PREFIX
> -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> -#else
> -#define SYMBOL_PREFIX ""
> -#endif
> -
>  /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
>  #ifdef CONFIG_FTRACE_MCOUNT_RECORD
>  # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 86e3285..0bd9d43 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -54,7 +54,7 @@ obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
>  obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
>  obj-$(CONFIG_UID16) += uid16.o
>  obj-$(CONFIG_MODULES) += module.o
> -obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o
> +obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
>  obj-$(CONFIG_KALLSYMS) += kallsyms.o
>  obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
>  obj-$(CONFIG_KEXEC) += kexec.o
> @@ -139,7 +139,7 @@ ifeq ($(CONFIG_MODULE_SIG),y)
>  extra_certificates:
>  	touch $@
>  
> -kernel/modsign_pubkey.o: signing_key.x509 extra_certificates
> +kernel/modsign_certificate.o: signing_key.x509 extra_certificates
>  
>  ###############################################################################
>  #
> diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
> new file mode 100644
> index 0000000..246b4c6
> --- /dev/null
> +++ b/kernel/modsign_certificate.S
> @@ -0,0 +1,19 @@
> +/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
> +#ifndef SYMBOL_PREFIX
> +#define ASM_SYMBOL(sym) sym
> +#else
> +#define PASTE2(x,y) x##y
> +#define PASTE(x,y) PASTE2(x,y)
> +#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
> +#endif
> +
> +#define GLOBAL(name)	\
> +	.globl ASM_SYMBOL(name);	\
> +	ASM_SYMBOL(name):
> +
> +	.section ".init.data","aw"
> +
> +GLOBAL(modsign_certificate_list)
> +	.incbin "signing_key.x509"
> +	.incbin "extra_certificates"
> +GLOBAL(modsign_certificate_list_end)
> diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
> index 767e559..045504f 100644
> --- a/kernel/modsign_pubkey.c
> +++ b/kernel/modsign_pubkey.c
> @@ -20,12 +20,6 @@ struct key *modsign_keyring;
>  
>  extern __initdata const u8 modsign_certificate_list[];
>  extern __initdata const u8 modsign_certificate_list_end[];
> -asm(".section .init.data,\"aw\"\n"
> -    SYMBOL_PREFIX "modsign_certificate_list:\n"
> -    ".incbin \"signing_key.x509\"\n"
> -    ".incbin \"extra_certificates\"\n"
> -    SYMBOL_PREFIX "modsign_certificate_list_end:"
> -    );
>  
>  /*
>   * We need to make sure ccache doesn't cache the .o file as it doesn't notice
>