Skip to content

Commit

Permalink
kexec, x86/purgatory: Unbreak it and clean it up
Browse files Browse the repository at this point in the history
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: 72042a8 ("x86/purgatory: Make functions and variables static")
Reported-by: Mike Galbraith <<[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Nicholas Mc Guire <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Vivek Goyal <[email protected]>
Cc: "Tobin C. Harding" <[email protected]>
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1703101315140.3681@nanos
Signed-off-by: Thomas Gleixner <[email protected]>
  • Loading branch information
KAGA-KOKO committed Mar 10, 2017
1 parent bba8376 commit 40c50c1
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 46 deletions.
12 changes: 6 additions & 6 deletions arch/powerpc/purgatory/trampoline.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 20 additions & 0 deletions arch/x86/include/asm/purgatory.h
Original file line number Diff line number Diff line change
@@ -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 */
9 changes: 6 additions & 3 deletions arch/x86/kernel/machine_kexec_64.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
35 changes: 17 additions & 18 deletions arch/x86/purgatory/purgatory.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions arch/x86/purgatory/purgatory.h

This file was deleted.

2 changes: 1 addition & 1 deletion arch/x86/purgatory/setup-x86_64.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion arch/x86/purgatory/sha256.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#ifndef SHA256_H
#define SHA256_H


#include <linux/types.h>
#include <crypto/sha.h>

Expand Down
23 changes: 23 additions & 0 deletions include/linux/purgatory.h
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions kernel/kexec_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 1 addition & 5 deletions kernel/kexec_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand Down

0 comments on commit 40c50c1

Please sign in to comment.