kexec, x86/purgatory: Cleanup the unholy mess

kexec, x86/purgatory: Cleanup the unholy mess

[Thomas Gleixner]

The purgatory code defines global variables which are referenced via a
symbol lookup in the kexec code (core and arch).

A recent commit addressing sparse warning made these static and thereby
broke kexec file.

Why did this happen? Simply because the whole machinery is undocumented and
lacks any form of forward declarations. The variable names are unspecific
and lack a prefix, so adding forward declarations creates shadow variables
in the core code. Aside of that the code relies on magic constants and
duplicate struct definitions with no way to ensure that these things stay
in sync.

Unbreak kexec and cleanup the mess by:

 - Adding proper forward declarations and document the usage
 - Use the proper common defines instead of magic constants
 - Add a purgatory_ prefix to have a proper name space
 - Use ARRAY_SIZE() instead of a homebrewn reimplementation

Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")
Reported-by: Mike Galbraith <<efault@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/powerpc/purgatory/trampoline.S |   12 ++++++------
 arch/x86/include/asm/purgatory.h    |   20 ++++++++++++++++++++
 arch/x86/kernel/machine_kexec_64.c  |    9 ++++++---
 arch/x86/purgatory/purgatory.c      |   35 +++++++++++++++++------------------
 arch/x86/purgatory/purgatory.h      |    8 --------
 arch/x86/purgatory/setup-x86_64.S   |    2 +-
 arch/x86/purgatory/sha256.h         |    1 -
 include/linux/purgatory.h           |   23 +++++++++++++++++++++++
 kernel/kexec_file.c                 |    8 ++++----
 kernel/kexec_internal.h             |    6 +-----
 10 files changed, 78 insertions(+), 46 deletions(-)

--- a/arch/powerpc/purgatory/trampoline.S
+++ b/arch/powerpc/purgatory/trampoline.S
@@ -116,13 +116,13 @@
 
 	.data
 	.balign 8
-.globl sha256_digest
-sha256_digest:
+.globl purgatory_sha256_digest
+purgatory_sha256_digest:
 	.skip	32
-	.size sha256_digest, . - sha256_digest
+	.size purgatory_sha256_digest, . - purgatory_sha256_digest
 
 	.balign 8
-.globl sha_regions
-sha_regions:
+.globl purgatory_sha_regions
+purgatory_sha_regions:
 	.skip	8 * 2 * 16
-	.size sha_regions, . - sha_regions
+	.size purgatory_sha_regions, . - purgatory_sha_regions
--- /dev/null
+++ b/arch/x86/include/asm/purgatory.h
@@ -0,0 +1,20 @@
+#ifndef _ASM_X86_PURGATORY_H
+#define _ASM_X86_PURGATORY_H
+
+#ifndef __ASSEMBLY__
+#include <linux/purgatory.h>
+
+extern void purgatory(void);
+/*
+ * These forward declarations serve two purposes:
+ *
+ * 1) Make sparse happy when checking arch/purgatory
+ * 2) Document that these are required to be global so the symbol
+ *    lookup in kexec works
+ */
+extern unsigned long purgatory_backup_dest;
+extern unsigned long purgatory_backup_src;
+extern unsigned long purgatory_backup_sz;
+#endif	/* __ASSEMBLY__ */
+
+#endif /* _ASM_PURGATORY_H */
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -194,19 +194,22 @@ static int arch_update_purgatory(struct
 
 	/* Setup copying of backup region */
 	if (image->type == KEXEC_TYPE_CRASH) {
-		ret = kexec_purgatory_get_set_symbol(image, "backup_dest",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_dest",
 				&image->arch.backup_load_addr,
 				sizeof(image->arch.backup_load_addr), 0);
 		if (ret)
 			return ret;
 
-		ret = kexec_purgatory_get_set_symbol(image, "backup_src",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_src",
 				&image->arch.backup_src_start,
 				sizeof(image->arch.backup_src_start), 0);
 		if (ret)
 			return ret;
 
-		ret = kexec_purgatory_get_set_symbol(image, "backup_sz",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_sz",
 				&image->arch.backup_src_sz,
 				sizeof(image->arch.backup_src_sz), 0);
 		if (ret)
--- a/arch/x86/purgatory/purgatory.c
+++ b/arch/x86/purgatory/purgatory.c
@@ -10,22 +10,19 @@
  * Version 2.  See the file COPYING for more details.
  */
 
+#include <linux/bug.h>
+#include <asm/purgatory.h>
+
 #include "sha256.h"
-#include "purgatory.h"
 #include "../boot/string.h"
 
-struct sha_region {
-	unsigned long start;
-	unsigned long len;
-};
-
-static unsigned long backup_dest;
-static unsigned long backup_src;
-static unsigned long backup_sz;
+unsigned long purgatory_backup_dest;
+unsigned long purgatory_backup_src;
+unsigned long purgatory_backup_sz;
 
-static u8 sha256_digest[SHA256_DIGEST_SIZE] = { 0 };
+u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE];
 
-struct sha_region sha_regions[16] = {};
+struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX];
 
 /*
  * On x86, second kernel requries first 640K of memory to boot. Copy
@@ -34,26 +31,28 @@ struct sha_region sha_regions[16] = {};
  */
 static int copy_backup_region(void)
 {
-	if (backup_dest)
-		memcpy((void *)backup_dest, (void *)backup_src, backup_sz);
-
+	if (purgatory_backup_dest) {
+		memcpy((void *)purgatory_backup_dest,
+		       (void *)purgatory_backup_src, purgatory_backup_sz);
+	}
 	return 0;
 }
 
 static int verify_sha256_digest(void)
 {
-	struct sha_region *ptr, *end;
+	struct kexec_sha_region *ptr, *end;
 	u8 digest[SHA256_DIGEST_SIZE];
 	struct sha256_state sctx;
 
 	sha256_init(&sctx);
-	end = &sha_regions[sizeof(sha_regions)/sizeof(sha_regions[0])];
-	for (ptr = sha_regions; ptr < end; ptr++)
+	end = purgatory_sha_regions + ARRAY_SIZE(purgatory_sha_regions);
+
+	for (ptr = purgatory_sha_regions; ptr < end; ptr++)
 		sha256_update(&sctx, (uint8_t *)(ptr->start), ptr->len);
 
 	sha256_final(&sctx, digest);
 
-	if (memcmp(digest, sha256_digest, sizeof(digest)))
+	if (memcmp(digest, purgatory_sha256_digest, sizeof(digest)))
 		return 1;
 
 	return 0;
--- a/arch/x86/purgatory/purgatory.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef PURGATORY_H
-#define PURGATORY_H
-
-#ifndef __ASSEMBLY__
-extern void purgatory(void);
-#endif	/* __ASSEMBLY__ */
-
-#endif /* PURGATORY_H */
--- a/arch/x86/purgatory/setup-x86_64.S
+++ b/arch/x86/purgatory/setup-x86_64.S
@@ -9,7 +9,7 @@
  * This source code is licensed under the GNU General Public License,
  * Version 2.  See the file COPYING for more details.
  */
-#include "purgatory.h"
+#include <asm/purgatory.h>
 
 	.text
 	.globl purgatory_start
--- a/arch/x86/purgatory/sha256.h
+++ b/arch/x86/purgatory/sha256.h
@@ -10,7 +10,6 @@
 #ifndef SHA256_H
 #define SHA256_H
 
-
 #include <linux/types.h>
 #include <crypto/sha.h>
 
--- /dev/null
+++ b/include/linux/purgatory.h
@@ -0,0 +1,23 @@
+#ifndef _LINUX_PURGATORY_H
+#define _LINUX_PURGATORY_H
+
+#include <linux/types.h>
+#include <crypto/sha.h>
+#include <uapi/linux/kexec.h>
+
+struct kexec_sha_region {
+	unsigned long start;
+	unsigned long len;
+};
+
+/*
+ * These forward declarations serve two purposes:
+ *
+ * 1) Make sparse happy when checking arch/purgatory
+ * 2) Document that these are required to be global so the symbol
+ *    lookup in kexec works
+ */
+extern struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX];
+extern u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE];
+
+#endif
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -614,13 +614,13 @@ static int kexec_calculate_store_digests
 		ret = crypto_shash_final(desc, digest);
 		if (ret)
 			goto out_free_digest;
-		ret = kexec_purgatory_get_set_symbol(image, "sha_regions",
-						sha_regions, sha_region_sz, 0);
+		ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
+						     sha_regions, sha_region_sz, 0);
 		if (ret)
 			goto out_free_digest;
 
-		ret = kexec_purgatory_get_set_symbol(image, "sha256_digest",
-						digest, SHA256_DIGEST_SIZE, 0);
+		ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha256_digest",
+						     digest, SHA256_DIGEST_SIZE, 0);
 		if (ret)
 			goto out_free_digest;
 	}
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -15,11 +15,7 @@ int kimage_is_destination_range(struct k
 extern struct mutex kexec_mutex;
 
 #ifdef CONFIG_KEXEC_FILE
-struct kexec_sha_region {
-	unsigned long start;
-	unsigned long len;
-};
-
+#include <linux/purgatory.h>
 void kimage_file_post_load_cleanup(struct kimage *image);
 #else /* CONFIG_KEXEC_FILE */
 static inline void kimage_file_post_load_cleanup(struct kimage *image) { }

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Mike Galbraith]

On Fri, 2017-03-10 at 13:17 +0100, Thomas Gleixner wrote:
> The purgatory code defines global variables which are referenced via a
> symbol lookup in the kexec code (core and arch).
> 
> A recent commit addressing sparse warning made these static and thereby
> broke kexec file.
> 
> Why did this happen? Simply because the whole machinery is undocumented and
> lacks any form of forward declarations. The variable names are unspecific
> and lack a prefix, so adding forward declarations creates shadow variables
> in the core code. Aside of that the code relies on magic constants and
> duplicate struct definitions with no way to ensure that these things stay
> in sync.
> 
> Unbreak kexec and cleanup the mess by:
> 
>  - Adding proper forward declarations and document the usage
>  - Use the proper common defines instead of magic constants
>  - Add a purgatory_ prefix to have a proper name space
>  - Use ARRAY_SIZE() instead of a homebrewn reimplementation
> 
> Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")

Well, almost fixes.

[   15.118820] kexec: symbol 'purgatory_sha_regions' in common section
[   15.119187] kexec-bzImage64: Loading purgatory failed

	-Mike

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Thomas Gleixner]

On Fri, 10 Mar 2017, Mike Galbraith wrote:
> On Fri, 2017-03-10 at 13:17 +0100, Thomas Gleixner wrote:
> > The purgatory code defines global variables which are referenced via a
> > symbol lookup in the kexec code (core and arch).
> > 
> > A recent commit addressing sparse warning made these static and thereby
> > broke kexec file.
> > 
> > Why did this happen? Simply because the whole machinery is undocumented and
> > lacks any form of forward declarations. The variable names are unspecific
> > and lack a prefix, so adding forward declarations creates shadow variables
> > in the core code. Aside of that the code relies on magic constants and
> > duplicate struct definitions with no way to ensure that these things stay
> > in sync.
> > 
> > Unbreak kexec and cleanup the mess by:
> > 
> >  - Adding proper forward declarations and document the usage
> >  - Use the proper common defines instead of magic constants
> >  - Add a purgatory_ prefix to have a proper name space
> >  - Use ARRAY_SIZE() instead of a homebrewn reimplementation
> > 
> > Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")
> 
> Well, almost fixes.
> 
> [   15.118820] kexec: symbol 'purgatory_sha_regions' in common section
> [   15.119187] kexec-bzImage64: Loading purgatory failed

Bah. /me goes to investigate.

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Vivek Goyal]

On Fri, Mar 10, 2017 at 02:57:38PM +0100, Thomas Gleixner wrote:
> On Fri, 10 Mar 2017, Mike Galbraith wrote:
> > On Fri, 2017-03-10 at 13:17 +0100, Thomas Gleixner wrote:
> > > The purgatory code defines global variables which are referenced via a
> > > symbol lookup in the kexec code (core and arch).
> > > 
> > > A recent commit addressing sparse warning made these static and thereby
> > > broke kexec file.
> > > 
> > > Why did this happen? Simply because the whole machinery is undocumented and
> > > lacks any form of forward declarations. The variable names are unspecific
> > > and lack a prefix, so adding forward declarations creates shadow variables
> > > in the core code. Aside of that the code relies on magic constants and
> > > duplicate struct definitions with no way to ensure that these things stay
> > > in sync.
> > > 
> > > Unbreak kexec and cleanup the mess by:
> > > 
> > >  - Adding proper forward declarations and document the usage
> > >  - Use the proper common defines instead of magic constants
> > >  - Add a purgatory_ prefix to have a proper name space
> > >  - Use ARRAY_SIZE() instead of a homebrewn reimplementation
> > > 
> > > Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")
> > 
> > Well, almost fixes.
> > 
> > [   15.118820] kexec: symbol 'purgatory_sha_regions' in common section
> > [   15.119187] kexec-bzImage64: Loading purgatory failed
> 
> Bah. /me goes to investigate.

I think we probably will have to initialize these global variables in
purgatory itself and that puts them in .data section and relocation
works.

That's how the code was intially. I initialized value of
purgatory_sha256_digest in the code and then did "readelf -a purgatory.o"
and symbol section index changed from COM to 3.

13: 0000000000000000    32 OBJECT  GLOBAL DEFAULT    3
purgatory_sha256_digest

[ 3] .data             PROGBITS         0000000000000000  00000120
         0000000000000020  0000000000000000  WA       0     0     32

Thanks
Vivek

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Mike Galbraith]

On Fri, 2017-03-10 at 14:57 +0100, Thomas Gleixner wrote:
> On Fri, 10 Mar 2017, Mike Galbraith wrote:
> > On Fri, 2017-03-10 at 13:17 +0100, Thomas Gleixner wrote:
> > > The purgatory code defines global variables which are referenced via a
> > > symbol lookup in the kexec code (core and arch).
> > > 
> > > A recent commit addressing sparse warning made these static and thereby
> > > broke kexec file.
> > > 
> > > Why did this happen? Simply because the whole machinery is undocumented and
> > > lacks any form of forward declarations. The variable names are unspecific
> > > and lack a prefix, so adding forward declarations creates shadow variables
> > > in the core code. Aside of that the code relies on magic constants and
> > > duplicate struct definitions with no way to ensure that these things stay
> > > in sync.
> > > 
> > > Unbreak kexec and cleanup the mess by:
> > > 
> > >  - Adding proper forward declarations and document the usage
> > >  - Use the proper common defines instead of magic constants
> > >  - Add a purgatory_ prefix to have a proper name space
> > >  - Use ARRAY_SIZE() instead of a homebrewn reimplementation
> > > 
> > > Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")
> > 
> > Well, almost fixes.
> > 
> > [   15.118820] kexec: symbol 'purgatory_sha_regions' in common section
> > [   15.119187] kexec-bzImage64: Loading purgatory failed
> 
> Bah. /me goes to investigate.

Stuffing the lot into .kexec-purgatory worked.

---
 arch/x86/purgatory/purgatory.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- a/arch/x86/purgatory/purgatory.c
+++ b/arch/x86/purgatory/purgatory.c
@@ -16,13 +16,13 @@
 #include "sha256.h"
 #include "../boot/string.h"
 
-unsigned long purgatory_backup_dest;
-unsigned long purgatory_backup_src;
-unsigned long purgatory_backup_sz;
+unsigned long purgatory_backup_dest __section(.kexec-purgatory);
+unsigned long purgatory_backup_src __section(.kexec-purgatory);
+unsigned long purgatory_backup_sz __section(.kexec-purgatory);
 
-u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE];
+u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(.kexec-purgatory);
 
-struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX];
+struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX] __section(.kexec-purgatory);
 
 /*
  * On x86, second kernel requries first 640K of memory to boot. Copy

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Thomas Gleixner]

On Fri, 10 Mar 2017, Mike Galbraith wrote:
> Stuffing the lot into .kexec-purgatory worked.

You beat me to it :)

And that's how that whole thing should have been done in the very beginning
instead of relying on uncomprehensible compile/link magics.

Thanks,

	tglx

> ---
>  arch/x86/purgatory/purgatory.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> --- a/arch/x86/purgatory/purgatory.c
> +++ b/arch/x86/purgatory/purgatory.c
> @@ -16,13 +16,13 @@
>  #include "sha256.h"
>  #include "../boot/string.h"
>  
> -unsigned long purgatory_backup_dest;
> -unsigned long purgatory_backup_src;
> -unsigned long purgatory_backup_sz;
> +unsigned long purgatory_backup_dest __section(.kexec-purgatory);
> +unsigned long purgatory_backup_src __section(.kexec-purgatory);
> +unsigned long purgatory_backup_sz __section(.kexec-purgatory);
>  
> -u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE];
> +u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(.kexec-purgatory);
>  
> -struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX];
> +struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX] __section(.kexec-purgatory);
>  
>  /*
>   * On x86, second kernel requries first 640K of memory to boot. Copy
> 

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Thomas Gleixner]

On Fri, 10 Mar 2017, Vivek Goyal wrote:
> I think we probably will have to initialize these global variables in
> purgatory itself and that puts them in .data section and relocation
> works.
> 
> That's how the code was intially. I initialized value of
> purgatory_sha256_digest in the code and then did "readelf -a purgatory.o"
> and symbol section index changed from COM to 3.
> 
> 13: 0000000000000000    32 OBJECT  GLOBAL DEFAULT    3
> purgatory_sha256_digest
> 
> [ 3] .data             PROGBITS         0000000000000000  00000120
>          0000000000000020  0000000000000000  WA       0     0     32

Yeah, and then instead of doing it proper you relied on compiler/link magic
which is unreliable, undocumented and uncomprehensible. But that's just
compatible to the rest of kexec. Works for me is never a good engineering
principle.

Thanks,

	tglx

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Mike Galbraith]

On Fri, 2017-03-10 at 15:56 +0100, Thomas Gleixner wrote:
> On Fri, 10 Mar 2017, Mike Galbraith wrote:
> > Stuffing the lot into .kexec-purgatory worked.
> 
> You beat me to it :)

That's odd, I'm usually a day late and a dollar short :)

	-Mike

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Vivek Goyal]

On Fri, Mar 10, 2017 at 03:58:20PM +0100, Thomas Gleixner wrote:
> On Fri, 10 Mar 2017, Vivek Goyal wrote:
> > I think we probably will have to initialize these global variables in
> > purgatory itself and that puts them in .data section and relocation
> > works.
> > 
> > That's how the code was intially. I initialized value of
> > purgatory_sha256_digest in the code and then did "readelf -a purgatory.o"
> > and symbol section index changed from COM to 3.
> > 
> > 13: 0000000000000000    32 OBJECT  GLOBAL DEFAULT    3
> > purgatory_sha256_digest
> > 
> > [ 3] .data             PROGBITS         0000000000000000  00000120
> >          0000000000000020  0000000000000000  WA       0     0     32
> 
> Yeah, and then instead of doing it proper you relied on compiler/link magic
> which is unreliable, undocumented and uncomprehensible. But that's just
> compatible to the rest of kexec. Works for me is never a good engineering
> principle.

Agreed. That was not a very good idea. Thanks for fixing this.

Vivek

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Thomas Gleixner]

On Fri, 10 Mar 2017, Mike Galbraith wrote:

> On Fri, 2017-03-10 at 15:56 +0100, Thomas Gleixner wrote:
> > On Fri, 10 Mar 2017, Mike Galbraith wrote:
> > > Stuffing the lot into .kexec-purgatory worked.
> > 
> > You beat me to it :)
> 
> That's odd, I'm usually a day late and a dollar short :)

Hehe. So I assume I can queue it with that fixup folded in?

Re: kexec, x86/purgatory: Cleanup the unholy mess

[Mike Galbraith]

On Fri, 2017-03-10 at 16:31 +0100, Thomas Gleixner wrote:
> On Fri, 10 Mar 2017, Mike Galbraith wrote:
> 
> > On Fri, 2017-03-10 at 15:56 +0100, Thomas Gleixner wrote:
> > > On Fri, 10 Mar 2017, Mike Galbraith wrote:
> > > > Stuffing the lot into .kexec-purgatory worked.
> > > 
> > > You beat me to it :)
> > 
> > That's odd, I'm usually a day late and a dollar short :)
> 
> Hehe. So I assume I can queue it with that fixup folded in?

Yeah, of course.

	-Mike

[tip:x86/urgent] kexec, x86/purgatory: Unbreak it and clean it up

[tip-bot for Thomas Gleixner]

Commit-ID:  40c50c1fecdf012a3bf055ec813f0ef2eda2749c
Gitweb:     http://git.kernel.org/tip/40c50c1fecdf012a3bf055ec813f0ef2eda2749c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 10 Mar 2017 13:17:18 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 10 Mar 2017 20:55:09 +0100

kexec, x86/purgatory: Unbreak it and clean it up

The purgatory code defines global variables which are referenced via a
symbol lookup in the kexec code (core and arch).

A recent commit addressing sparse warnings made these static and thereby
broke kexec_file.

Why did this happen? Simply because the whole machinery is undocumented and
lacks any form of forward declarations. The variable names are unspecific
and lack a prefix, so adding forward declarations creates shadow variables
in the core code. Aside of that the code relies on magic constants and
duplicate struct definitions with no way to ensure that these things stay
in sync. The section placement of the purgatory variables happened by
chance and not by design.

Unbreak kexec and cleanup the mess:

 - Add proper forward declarations and document the usage
 - Use common struct definition
 - Use the proper common defines instead of magic constants
 - Add a purgatory_ prefix to have a proper name space
 - Use ARRAY_SIZE() instead of a homebrewn reimplementation
 - Add proper sections to the purgatory variables [ From Mike ]

Fixes: 72042a8c7b01 ("x86/purgatory: Make functions and variables static")
Reported-by: Mike Galbraith <<efault@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Nicholas Mc Guire <der.herr@hofr.at>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: "Tobin C. Harding" <me@tobin.cc>
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1703101315140.3681@nanos
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/powerpc/purgatory/trampoline.S | 12 ++++++------
 arch/x86/include/asm/purgatory.h    | 20 ++++++++++++++++++++
 arch/x86/kernel/machine_kexec_64.c  |  9 ++++++---
 arch/x86/purgatory/purgatory.c      | 35 +++++++++++++++++------------------
 arch/x86/purgatory/purgatory.h      |  8 --------
 arch/x86/purgatory/setup-x86_64.S   |  2 +-
 arch/x86/purgatory/sha256.h         |  1 -
 include/linux/purgatory.h           | 23 +++++++++++++++++++++++
 kernel/kexec_file.c                 |  8 ++++----
 kernel/kexec_internal.h             |  6 +-----
 10 files changed, 78 insertions(+), 46 deletions(-)

diff --git a/arch/powerpc/purgatory/trampoline.S b/arch/powerpc/purgatory/trampoline.S
index f9760cc..3696ea6 100644
--- a/arch/powerpc/purgatory/trampoline.S
+++ b/arch/powerpc/purgatory/trampoline.S
@@ -116,13 +116,13 @@ dt_offset:
 
 	.data
 	.balign 8
-.globl sha256_digest
-sha256_digest:
+.globl purgatory_sha256_digest
+purgatory_sha256_digest:
 	.skip	32
-	.size sha256_digest, . - sha256_digest
+	.size purgatory_sha256_digest, . - purgatory_sha256_digest
 
 	.balign 8
-.globl sha_regions
-sha_regions:
+.globl purgatory_sha_regions
+purgatory_sha_regions:
 	.skip	8 * 2 * 16
-	.size sha_regions, . - sha_regions
+	.size purgatory_sha_regions, . - purgatory_sha_regions
diff --git a/arch/x86/include/asm/purgatory.h b/arch/x86/include/asm/purgatory.h
new file mode 100644
index 0000000..d7da272
--- /dev/null
+++ b/arch/x86/include/asm/purgatory.h
@@ -0,0 +1,20 @@
+#ifndef _ASM_X86_PURGATORY_H
+#define _ASM_X86_PURGATORY_H
+
+#ifndef __ASSEMBLY__
+#include <linux/purgatory.h>
+
+extern void purgatory(void);
+/*
+ * These forward declarations serve two purposes:
+ *
+ * 1) Make sparse happy when checking arch/purgatory
+ * 2) Document that these are required to be global so the symbol
+ *    lookup in kexec works
+ */
+extern unsigned long purgatory_backup_dest;
+extern unsigned long purgatory_backup_src;
+extern unsigned long purgatory_backup_sz;
+#endif	/* __ASSEMBLY__ */
+
+#endif /* _ASM_PURGATORY_H */
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 307b1f4..857cdbd 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -194,19 +194,22 @@ static int arch_update_purgatory(struct kimage *image)
 
 	/* Setup copying of backup region */
 	if (image->type == KEXEC_TYPE_CRASH) {
-		ret = kexec_purgatory_get_set_symbol(image, "backup_dest",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_dest",
 				&image->arch.backup_load_addr,
 				sizeof(image->arch.backup_load_addr), 0);
 		if (ret)
 			return ret;
 
-		ret = kexec_purgatory_get_set_symbol(image, "backup_src",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_src",
 				&image->arch.backup_src_start,
 				sizeof(image->arch.backup_src_start), 0);
 		if (ret)
 			return ret;
 
-		ret = kexec_purgatory_get_set_symbol(image, "backup_sz",
+		ret = kexec_purgatory_get_set_symbol(image,
+				"purgatory_backup_sz",
 				&image->arch.backup_src_sz,
 				sizeof(image->arch.backup_src_sz), 0);
 		if (ret)
diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c
index b6d5c89..470edad 100644
--- a/arch/x86/purgatory/purgatory.c
+++ b/arch/x86/purgatory/purgatory.c
@@ -10,22 +10,19 @@
  * Version 2.  See the file COPYING for more details.
  */
 
+#include <linux/bug.h>
+#include <asm/purgatory.h>
+
 #include "sha256.h"
-#include "purgatory.h"
 #include "../boot/string.h"
 
-struct sha_region {
-	unsigned long start;
-	unsigned long len;
-};
-
-static unsigned long backup_dest;
-static unsigned long backup_src;
-static unsigned long backup_sz;
+unsigned long purgatory_backup_dest __section(.kexec-purgatory);
+unsigned long purgatory_backup_src __section(.kexec-purgatory);
+unsigned long purgatory_backup_sz __section(.kexec-purgatory);
 
-static u8 sha256_digest[SHA256_DIGEST_SIZE] = { 0 };
+u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(.kexec-purgatory);
 
-struct sha_region sha_regions[16] = {};
+struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX] __section(.kexec-purgatory);
 
 /*
  * On x86, second kernel requries first 640K of memory to boot. Copy
@@ -34,26 +31,28 @@ struct sha_region sha_regions[16] = {};
  */
 static int copy_backup_region(void)
 {
-	if (backup_dest)
-		memcpy((void *)backup_dest, (void *)backup_src, backup_sz);
-
+	if (purgatory_backup_dest) {
+		memcpy((void *)purgatory_backup_dest,
+		       (void *)purgatory_backup_src, purgatory_backup_sz);
+	}
 	return 0;
 }
 
 static int verify_sha256_digest(void)
 {
-	struct sha_region *ptr, *end;
+	struct kexec_sha_region *ptr, *end;
 	u8 digest[SHA256_DIGEST_SIZE];
 	struct sha256_state sctx;
 
 	sha256_init(&sctx);
-	end = &sha_regions[sizeof(sha_regions)/sizeof(sha_regions[0])];
-	for (ptr = sha_regions; ptr < end; ptr++)
+	end = purgatory_sha_regions + ARRAY_SIZE(purgatory_sha_regions);
+
+	for (ptr = purgatory_sha_regions; ptr < end; ptr++)
 		sha256_update(&sctx, (uint8_t *)(ptr->start), ptr->len);
 
 	sha256_final(&sctx, digest);
 
-	if (memcmp(digest, sha256_digest, sizeof(digest)))
+	if (memcmp(digest, purgatory_sha256_digest, sizeof(digest)))
 		return 1;
 
 	return 0;
diff --git a/arch/x86/purgatory/purgatory.h b/arch/x86/purgatory/purgatory.h
deleted file mode 100644
index e2e365a..0000000
--- a/arch/x86/purgatory/purgatory.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef PURGATORY_H
-#define PURGATORY_H
-
-#ifndef __ASSEMBLY__
-extern void purgatory(void);
-#endif	/* __ASSEMBLY__ */
-
-#endif /* PURGATORY_H */
diff --git a/arch/x86/purgatory/setup-x86_64.S b/arch/x86/purgatory/setup-x86_64.S
index f90e9df..dfae9b9 100644
--- a/arch/x86/purgatory/setup-x86_64.S
+++ b/arch/x86/purgatory/setup-x86_64.S
@@ -9,7 +9,7 @@
  * This source code is licensed under the GNU General Public License,
  * Version 2.  See the file COPYING for more details.
  */
-#include "purgatory.h"
+#include <asm/purgatory.h>
 
 	.text
 	.globl purgatory_start
diff --git a/arch/x86/purgatory/sha256.h b/arch/x86/purgatory/sha256.h
index bd15a41..2867d98 100644
--- a/arch/x86/purgatory/sha256.h
+++ b/arch/x86/purgatory/sha256.h
@@ -10,7 +10,6 @@
 #ifndef SHA256_H
 #define SHA256_H
 
-
 #include <linux/types.h>
 #include <crypto/sha.h>
 
diff --git a/include/linux/purgatory.h b/include/linux/purgatory.h
new file mode 100644
index 0000000..d60d4e278
--- /dev/null
+++ b/include/linux/purgatory.h
@@ -0,0 +1,23 @@
+#ifndef _LINUX_PURGATORY_H
+#define _LINUX_PURGATORY_H
+
+#include <linux/types.h>
+#include <crypto/sha.h>
+#include <uapi/linux/kexec.h>
+
+struct kexec_sha_region {
+	unsigned long start;
+	unsigned long len;
+};
+
+/*
+ * These forward declarations serve two purposes:
+ *
+ * 1) Make sparse happy when checking arch/purgatory
+ * 2) Document that these are required to be global so the symbol
+ *    lookup in kexec works
+ */
+extern struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX];
+extern u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE];
+
+#endif
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b56a558..b118735 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -614,13 +614,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
 		ret = crypto_shash_final(desc, digest);
 		if (ret)
 			goto out_free_digest;
-		ret = kexec_purgatory_get_set_symbol(image, "sha_regions",
-						sha_regions, sha_region_sz, 0);
+		ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha_regions",
+						     sha_regions, sha_region_sz, 0);
 		if (ret)
 			goto out_free_digest;
 
-		ret = kexec_purgatory_get_set_symbol(image, "sha256_digest",
-						digest, SHA256_DIGEST_SIZE, 0);
+		ret = kexec_purgatory_get_set_symbol(image, "purgatory_sha256_digest",
+						     digest, SHA256_DIGEST_SIZE, 0);
 		if (ret)
 			goto out_free_digest;
 	}
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index 4cef7e4..799a8a4 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -15,11 +15,7 @@ int kimage_is_destination_range(struct kimage *image,
 extern struct mutex kexec_mutex;
 
 #ifdef CONFIG_KEXEC_FILE
-struct kexec_sha_region {
-	unsigned long start;
-	unsigned long len;
-};
-
+#include <linux/purgatory.h>
 void kimage_file_post_load_cleanup(struct kimage *image);
 #else /* CONFIG_KEXEC_FILE */
 static inline void kimage_file_post_load_cleanup(struct kimage *image) { }