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>

diff --git a/arch/powerpc/purgatory/trampoline.S b/arch/powerpc/purgatory/trampoline.S
index f9760ccf40323674cecb3f4f14cc98aa8e615cfe..3696ea6c4826b9740398113d207b1679318db2f8 100644 (file)
--- 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 (file)
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 307b1f4543de4bc96c6759c5f81a7faf5c9f443c..857cdbd028675716afad71c0b48974399889e622 100644 (file)
--- 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 b6d5c8946e664aad672d15c3058e214c8192b8c6..470edad96bb9560a218affd4c0922888f9200dba 100644 (file)
--- 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 (file)
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 f90e9dfa90bb928979698d46af465e537f8314a2..dfae9b9e60b5ba01e62d92d442bd1276eee45ae7 100644 (file)
--- 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 bd15a4127735e5f6ed9560b8dfb2503126f56e47..2867d9825a57e5f1f734bfb4a5777bc31810b090 100644 (file)
--- 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 (file)
index 0000000..d60d4e2
--- /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 b56a558e406db6375bea4b07e5873a4c6f0b401e..b118735fea9da471a15ba627c87af523b891bafa 100644 (file)
--- 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 4cef7e4706b098d7918b53ff1e1b931d1a5ec8dc..799a8a4521870a6444818fef64c0ae1e2dfad671 100644 (file)
--- 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) { }