Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map
On Wed, Jan 16, 2019 at 7:10 AM Borislav Petkov wrote:
> > +#ifdef CONFIG_ACPI
> > + /* Setup ACPI RSDP pointer in case EFI is not available in second
> > kernel */
> > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) ||
> > efi_enabled(EFI_OLD_MEMMAP))) {
> > + /* Copied from acpi_os_get_root_pointer accordingly */
> > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr;
> > + if (!params->acpi_rsdp_addr) {
> > + if (efi_enabled(EFI_CONFIG_TABLES)) {
> > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
> > + params->acpi_rsdp_addr = efi.acpi20;
> > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
> > + params->acpi_rsdp_addr = efi.acpi;
> > + } else if
> > (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
> > +
> > acpi_find_root_pointer(>acpi_rsdp_addr);
> > + }
> > + }
> > + if (!params->acpi_rsdp_addr)
> > + pr_warn("RSDP is not available for second kernel\n");
> > + }
> > #endif
>
> Amazing the amount of ACPI RDSP parsing and fiddling patches flying
> around these days...
>
> In any case, this needs to be synchronized with:
>
> https://lkml.kernel.org/r/20190107032243.25324-1-fanc.f...@cn.fujitsu.com
>
> and checked whether the above can be used instead of sprinkling of ACPI
> parsing code left and right.
>
> Thx.
Hi thanks for the suggestion.
I didn't see a way to reuse things in that patch series, situation is
different, in that patch it needs to get RSDP in very early boot stage
so it did everything from scratch, in this patch kexec_file_load need
to get RSDP too, but everything is well setup so things are a lot
easier, just read from current boot_prams, efi and fallback to
acpi_find_root_pointer should be good.
And that patch series also need to consider boot_params.acpi_rsdp_addr
value, or it won't work if the system is rebooted with kexec, efi is
disabled and acpi_rsdp is provided by boot_params (Xen PVH guests also
need to get acpi_rsdp from boot_params according to commit message of
ae7e1238e68f2a472a125673ab506d49158c1889). Will add some comment and
discuss.
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
--
Best Regards,
Kairui Song
Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map
On Wed, Jan 16, 2019 at 11:32 AM Dave Young wrote:
>
> On 01/16/19 at 12:10am, Borislav Petkov wrote:
> > On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote:
> > > When efi=noruntime or efi=oldmap is used, EFI services won't be available
> > > in the second kernel, therefore the second kernel will not be able to get
> > > the ACPI RSDP address from firmware by calling EFI services and won't
> > > boot. Previously we are expecting the user to set the acpi_rsdp=
> > > on kernel command line for second kernel as there was no way to pass RSDP
> > > address to second kernel.
> > >
> > > After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from
> > > boot params if available'), now it's possible to set an acpi_rsdp_addr
> > > parameter in the boot_params passed to second kernel, this commit make
> > > use of it, detect and set the RSDP address when it's required for second
> > > kernel to boot.
> > >
> > > Tested with an EFI enabled KVM VM with efi=noruntime.
> > >
> > > Suggested-by: Dave Young
> > > Signed-off-by: Kairui Song
> > > ---
> > > arch/x86/kernel/kexec-bzimage64.c | 21 +
> > > drivers/acpi/acpica/tbxfroot.c| 3 +--
> > > include/acpi/acpixf.h | 2 +-
> > > 3 files changed, 23 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/kexec-bzimage64.c
> > > b/arch/x86/kernel/kexec-bzimage64.c
> > > index 53917a3ebf94..0a90dcbd041f 100644
> > > --- a/arch/x86/kernel/kexec-bzimage64.c
> > > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > > @@ -20,6 +20,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > >
> > > #include
> > > #include
> > > @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct
> > > boot_params *params,
> > > /* Setup EFI state */
> > > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> > > efi_setup_data_offset);
> > > +
> > > +#ifdef CONFIG_ACPI
> > > + /* Setup ACPI RSDP pointer in case EFI is not available in second
> > > kernel */
> > > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) ||
> > > efi_enabled(EFI_OLD_MEMMAP))) {
> > > + /* Copied from acpi_os_get_root_pointer accordingly */
> > > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr;
> > > + if (!params->acpi_rsdp_addr) {
> > > + if (efi_enabled(EFI_CONFIG_TABLES)) {
> > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
> > > + params->acpi_rsdp_addr = efi.acpi20;
> > > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
> > > + params->acpi_rsdp_addr = efi.acpi;
> > > + } else if
> > > (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
> > > +
> > > acpi_find_root_pointer(>acpi_rsdp_addr);
> > > + }
> > > + }
> > > + if (!params->acpi_rsdp_addr)
> > > + pr_warn("RSDP is not available for second kernel\n");
> > > + }
> > > #endif
> >
> > Amazing the amount of ACPI RDSP parsing and fiddling patches flying
> > around these days...
> >
> > In any case, this needs to be synchronized with:
> >
> > https://lkml.kernel.org/r/20190107032243.25324-1-fanc.f...@cn.fujitsu.com
> >
> > and checked whether the above can be used instead of sprinkling of ACPI
> > parsing code left and right.
>
> Both Baoquan and Chao are cced for comments.
> The above KASLR patches seems some early code to parsing acpi, but I think in
> this
> patch just call acpi function to get the root pointer no need to add the
> duplicate logic of if/else/else if.
>
> Kairui, do you have any reason for the checking? Is there some simple
> acpi function to just return the root pointer?
Hi, I'm afraid that would require moving multiple structure and
function out of .init,
acpi_os_get_root_pointer is an ideal function to do the job, but it's
in .init and (on x86) it will call x86_init.acpi.get_root_pointer (by
calling acpi_arch_get_root_pointer) which will also be freed after
init, unless I change the x86_init, move they out of .init which is
not a good idea.
Maybe I could split acpi_os_get_root_pointer into two, one gets freed
after init, one for later use. But the "acpi_rsdp_addr" trick only
works for x86, and this would change more common acpi driver code so
not sure if a good idea.
>
> >
> > Thx.
> >
> > --
> > Regards/Gruss,
> > Boris.
> >
> > Good mailing practices for 400: avoid top-posting and trim the reply.
>
> Thanks
> Dave
--
Best Regards,
Kairui Song
Re: [RFC PATCH v2 2/2] kexec, KEYS: Make use of platform keyring for signature verify
On Tue, Jan 15, 2019 at 11:47 PM Mimi Zohar wrote:
>
> On Tue, 2019-01-15 at 17:45 +0800, Kairui Song wrote:
>
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c
> > b/arch/x86/kernel/kexec-bzimage64.c
> > index 7d97e432cbbc..a06b04065bb1 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -534,9 +534,18 @@ static int bzImage64_cleanup(void *loader_data)
> > #ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
> > static int bzImage64_verify_sig(const char *kernel, unsigned long
> > kernel_len)
> > {
> > - return verify_pefile_signature(kernel, kernel_len,
> > -VERIFY_USE_SECONDARY_KEYRING,
> > -VERIFYING_KEXEC_PE_SIGNATURE);
> > + int ret;
> > + ret = verify_pefile_signature(kernel, kernel_len,
> > + VERIFY_USE_SECONDARY_KEYRING,
> > + VERIFYING_KEXEC_PE_SIGNATURE);
> > +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
>
> Consider using IS_ENABLED() or IS_BUILTIN().
>
> Mimi
Thanks for the suggestion, will update the patch later if there are no
other comments.
>
> > + if (ret == -ENOKEY) {
> > + ret = verify_pefile_signature(kernel, kernel_len,
> > + VERIFY_USE_PLATFORM_KEYRING,
> > + VERIFYING_KEXEC_PE_SIGNATURE);
> > + }
> > +#endif
> > + return ret;
> > }
> > #endif
>
--
Best Regards,
Kairui Song
Re: [RFC PATCH v2 1/2] integrity, KEYS: add a reference to platform keyring
On Tue, Jan 15, 2019 at 11:34 PM Mimi Zohar wrote:
>
> On Tue, 2019-01-15 at 17:45 +0800, Kairui Song wrote:
> [snip]
>
> > diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> > index f45d6edecf99..bfabc2a8111d 100644
> > --- a/security/integrity/digsig.c
> > +++ b/security/integrity/digsig.c
> > @@ -89,6 +89,12 @@ static int __integrity_init_keyring(const unsigned int
> > id, key_perm_t perm,
> > keyring[id] = NULL;
> > }
> >
> > +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> > + if (id == INTEGRITY_KEYRING_PLATFORM) {
> > + set_platform_trusted_keys(keyring[id]);
> > + }
> > +#endif
> > +
> > return err;
> > }
> >
>
> Any reason for setting it here as opposed to in the caller
> platform_keyring_init()?
>
> Mimi
>
Yes, "keyring" is static so unless I expose it to other files, it is
only accessible here. And I think there should be no problem to put
the set_platform_trusted_keys here.
--
Best Regards,
Kairui Song
[PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map
When efi=noruntime or efi=oldmap is used, EFI services won't be available
in the second kernel, therefore the second kernel will not be able to get
the ACPI RSDP address from firmware by calling EFI services and won't
boot. Previously we are expecting the user to set the acpi_rsdp=
on kernel command line for second kernel as there was no way to pass RSDP
address to second kernel.
After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from
boot params if available'), now it's possible to set an acpi_rsdp_addr
parameter in the boot_params passed to second kernel, this commit make
use of it, detect and set the RSDP address when it's required for second
kernel to boot.
Tested with an EFI enabled KVM VM with efi=noruntime.
Suggested-by: Dave Young
Signed-off-by: Kairui Song
---
arch/x86/kernel/kexec-bzimage64.c | 21 +
drivers/acpi/acpica/tbxfroot.c| 3 +--
include/acpi/acpixf.h | 2 +-
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/kexec-bzimage64.c
b/arch/x86/kernel/kexec-bzimage64.c
index 53917a3ebf94..0a90dcbd041f 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct
boot_params *params,
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
+
+#ifdef CONFIG_ACPI
+ /* Setup ACPI RSDP pointer in case EFI is not available in second
kernel */
+ if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) ||
efi_enabled(EFI_OLD_MEMMAP))) {
+ /* Copied from acpi_os_get_root_pointer accordingly */
+ params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr;
+ if (!params->acpi_rsdp_addr) {
+ if (efi_enabled(EFI_CONFIG_TABLES)) {
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ params->acpi_rsdp_addr = efi.acpi20;
+ else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
+ params->acpi_rsdp_addr = efi.acpi;
+ } else if
(IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
+ acpi_find_root_pointer(>acpi_rsdp_addr);
+ }
+ }
+ if (!params->acpi_rsdp_addr)
+ pr_warn("RSDP is not available for second kernel\n");
+ }
#endif
+#endif
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
diff --git a/drivers/acpi/acpica/tbxfroot.c b/drivers/acpi/acpica/tbxfroot.c
index 483d0ce5180a..dac1e34a931c 100644
--- a/drivers/acpi/acpica/tbxfroot.c
+++ b/drivers/acpi/acpica/tbxfroot.c
@@ -108,8 +108,7 @@ acpi_status acpi_tb_validate_rsdp(struct acpi_table_rsdp
*rsdp)
*
**/
-acpi_status ACPI_INIT_FUNCTION
-acpi_find_root_pointer(acpi_physical_address *table_address)
+acpi_status acpi_find_root_pointer(acpi_physical_address *table_address)
{
u8 *table_ptr;
u8 *mem_rover;
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 7aa38b648564..869d75ecaf7d 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -474,7 +474,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
acpi_reallocate_root_table(void))
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
+ACPI_EXTERNAL_RETURN_STATUS(acpi_status
acpi_find_root_pointer(acpi_physical_address
*rsdp_address))
ACPI_EXTERNAL_RETURN_STATUS(acpi_status
--
2.20.1
[PATCH v2 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled
Currently with "efi=noruntime" in kernel command line, calling kexec_file_load will raise below problem: [ 97.967067] BUG: unable to handle kernel NULL pointer dereference at [ 97.967894] #PF error: [normal kernel read fault] ... [ 97.980456] Call Trace: [ 97.980724] efi_runtime_map_copy+0x28/0x30 [ 97.981267] bzImage64_load+0x688/0x872 [ 97.981794] arch_kexec_kernel_image_load+0x6d/0x70 [ 97.982441] kimage_file_alloc_init+0x13e/0x220 [ 97.983035] __x64_sys_kexec_file_load+0x144/0x290 [ 97.983586] do_syscall_64+0x55/0x1a0 [ 97.983962] entry_SYSCALL_64_after_hwframe+0x44/0xa9 When efi runtime is not enabled, efi memmap is not mapped, so just skip EFI info setup. Suggested-by: Dave Young Signed-off-by: Kairui Song --- arch/x86/kernel/kexec-bzimage64.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 0d5efa34f359..53917a3ebf94 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -167,6 +167,9 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, struct efi_info *current_ei = _params.efi_info; struct efi_info *ei = >efi_info; + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return 0; + if (!current_ei->efi_memmap_size) return 0; -- 2.20.1
[PATCH 0/2] make kexec work with efi=noruntime or efi=old_map
This patch series fix the kexec panic on efi=noruntime or efi=old_map and leverage acpi_rsdp_addr to make the second kernel boot up properly. Kairui Song (2): x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Update from V1: - Add a cover letter and fix some type in commit message - Previous patches are not sent in a single thread arch/x86/kernel/kexec-bzimage64.c | 24 drivers/acpi/acpica/tbxfroot.c| 3 +-- include/acpi/acpixf.h | 2 +- 3 files changed, 26 insertions(+), 3 deletions(-) -- 2.20.1
[RFC PATCH v2 2/2] kexec, KEYS: Make use of platform keyring for signature verify
With KEXEC_BZIMAGE_VERIFY_SIG enabled, kexec_file_load will need to
verify the kernel image. The image might be signed with third part keys,
and the keys could be stored in firmware, then got loaded into the
.platform keyring. Now we have a symbol .platform_trusted_keyring as the
reference to .platform keyring, this patch makes use if it and allow
kexec_file_load to verify the image against keys in .platform keyring.
This commit adds a VERIFY_USE_PLATFORM_KEYRING similar to previous
VERIFY_USE_SECONDARY_KEYRING indicating that verify_pkcs7_signature
should verify the signature using platform keyring. Also, decrease
the error message log level when verification failed with -ENOKEY,
so that if called tried multiple time with different keyring it
won't generate extra noises.
Signed-off-by: Kairui Song
---
arch/x86/kernel/kexec-bzimage64.c | 15 ---
certs/system_keyring.c| 11 ++-
include/linux/verification.h | 3 +++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/kexec-bzimage64.c
b/arch/x86/kernel/kexec-bzimage64.c
index 7d97e432cbbc..a06b04065bb1 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -534,9 +534,18 @@ static int bzImage64_cleanup(void *loader_data)
#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
{
- return verify_pefile_signature(kernel, kernel_len,
- VERIFY_USE_SECONDARY_KEYRING,
- VERIFYING_KEXEC_PE_SIGNATURE);
+ int ret;
+ ret = verify_pefile_signature(kernel, kernel_len,
+ VERIFY_USE_SECONDARY_KEYRING,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+ if (ret == -ENOKEY) {
+ ret = verify_pefile_signature(kernel, kernel_len,
+ VERIFY_USE_PLATFORM_KEYRING,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+ }
+#endif
+ return ret;
}
#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 4690ef9cda8a..df2a09599bd8 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -239,12 +239,21 @@ int verify_pkcs7_signature(const void *data, size_t len,
trusted_keys = secondary_trusted_keys;
#else
trusted_keys = builtin_trusted_keys;
+#endif
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+ } else if (trusted_keys == VERIFY_USE_PLATFORM_KEYRING) {
+ trusted_keys = platform_trusted_keys;
+ if (!trusted_keys) {
+ ret = -ENOKEY;
+ pr_devel("PKCS#7 platform keyring is not available\n");
+ goto error;
+ }
#endif
}
ret = pkcs7_validate_trust(pkcs7, trusted_keys);
if (ret < 0) {
if (ret == -ENOKEY)
- pr_err("PKCS#7 signature not signed with a trusted
key\n");
+ pr_devel("PKCS#7 signature not signed with a trusted
key\n");
goto error;
}
diff --git a/include/linux/verification.h b/include/linux/verification.h
index cfa4730d607a..dbd1686e9bc4 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -17,6 +17,9 @@
* should be used.
*/
#define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+#define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)
+#endif
/*
* The use to which an asymmetric key is being put.
--
2.20.1
[RFC PATCH v2 0/2] let kexec_file_load use platform keyring to verify the kernel image
Hi, This patch series adds a .platform_trusted_keys in system_keyring as the reference to .platform keyring in integrity subsystem, when platform keyring is being initialized it will be updated. So other component could use this keyring as well. This patch series also let kexec_file_load use platform keyring as fall back if it failed to verify the image against secondary keyring, make it possible to load kernel signed by third part key if third party key is imported in the firmware. After this patch kexec_file_load will be able to verify a signed PE bzImage using keys in platform keyring. Tested in a VM with locally signed kernel with pesign and imported the cert to EFI's MokList variable. Kairui Song (2): integrity, KEYS: add a reference to platform keyring kexec, KEYS: Make use of platform keyring for signature verify Update from V1: - Make platform_trusted_keys static, and update commit message as suggested by Mimi Zohar - Always check if platform keyring is initialized before use it arch/x86/kernel/kexec-bzimage64.c | 15 --- certs/system_keyring.c| 20 +++- include/keys/system_keyring.h | 5 + include/linux/verification.h | 3 +++ security/integrity/digsig.c | 6 ++ 5 files changed, 45 insertions(+), 4 deletions(-) -- 2.20.1
[RFC PATCH v2 1/2] integrity, KEYS: add a reference to platform keyring
Currently when loading new kernel via kexec_file_load syscall, it is able
to verify the signed PE bzimage against .builtin_trusted_keys or
.secondary_trusted_keys. But the image could be signed with third part
keys which will be provided by platform or firmware as EFI variable (eg.
stored in MokListRT EFI variable), and the keys won't be available in
keyrings mentioned above.
After commit 9dc92c45177a ('integrity: Define a trusted platform keyring')
a .platform keyring is introduced to store the keys provided by platform
or firmware, this keyring is intended to be used for verifying kernel
images being loaded by kexec_file_load syscall. And with a few following
up commits, keys provided by firmware is being loaded into this keyring,
and IMA-appraisal is able to use the keyring to verify kernel images.
IMA is the currently the only user of that keyring.
This patch exposes the .platform, and makes it useable for other
components. For example, kexec_file_load could use this .platform
keyring to verify the kernel image's image.
Suggested-by: Mimi Zohar
Signed-off-by: Kairui Song
---
certs/system_keyring.c| 9 +
include/keys/system_keyring.h | 5 +
security/integrity/digsig.c | 6 ++
3 files changed, 20 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 81728717523d..4690ef9cda8a 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -24,6 +24,9 @@ static struct key *builtin_trusted_keys;
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
static struct key *secondary_trusted_keys;
#endif
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+static struct key *platform_trusted_keys;
+#endif
extern __initconst const u8 system_certificate_list[];
extern __initconst const unsigned long system_certificate_list_size;
@@ -265,4 +268,10 @@ int verify_pkcs7_signature(const void *data, size_t len,
}
EXPORT_SYMBOL_GPL(verify_pkcs7_signature);
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+void __init set_platform_trusted_keys(struct key *keyring) {
+ platform_trusted_keys = keyring;
+}
+#endif
+
#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 359c2f936004..9e1b7849b6aa 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -61,5 +61,10 @@ static inline struct key *get_ima_blacklist_keyring(void)
}
#endif /* CONFIG_IMA_BLACKLIST_KEYRING */
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+
+extern void __init set_platform_trusted_keys(struct key* keyring);
+
+#endif /* CONFIG_INTEGRITY_PLATFORM_KEYRING */
#endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index f45d6edecf99..bfabc2a8111d 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -89,6 +89,12 @@ static int __integrity_init_keyring(const unsigned int id,
key_perm_t perm,
keyring[id] = NULL;
}
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+ if (id == INTEGRITY_KEYRING_PLATFORM) {
+ set_platform_trusted_keys(keyring[id]);
+ }
+#endif
+
return err;
}
--
2.20.1
Re: [RFC PATCH 2/2] kexec, KEYS: Make use of platform keyring for signature verify
On Tue, Jan 15, 2019 at 10:42 AM Dave Young wrote:
>
> On 01/14/19 at 11:10am, Mimi Zohar wrote:
> > On Sun, 2019-01-13 at 09:39 +0800, Dave Young wrote:
> > > Hi,
> > >
> > > On 01/11/19 at 11:13am, Mimi Zohar wrote:
> > > > On Fri, 2019-01-11 at 21:43 +0800, Dave Young wrote:
> > > > [snip]
> > > >
> > > > > Personally I would like to see platform key separated from integrity.
> > > > > But for the kexec_file part I think it is good at least it works with
> > > > > this fix.
> > > > >
> > > > > Acked-by: Dave Young
> > > >
> > > > The original "platform" keyring patches that Nayna posted multiple
> > > > times were in the certs directory, but nobody commented/responded. So
> > > > she reworked the patches, moving them to the integrity directory and
> > > > posted them (cc'ing the kexec mailing list). It's a bit late to be
> > > > asking to move it, isn't it?
> > >
> > > Hmm, apologize for being late, I did not get chance to have a look the
> > > old series. Since we have the needs now, it should be still fine
> > >
> > > Maybe Kairui can check Nayna's old series, see if he can do something
> > > again?
> >
> > Whether the platform keyring is defined in certs/ or in integrity/ the
> > keyring id needs to be accessible to the other, without making the
> > keyring id global. Moving where the platform keyring is defined is
> > not the problem.
>
> Agreed, but just feel kexec depends on IMA sounds not good.
>
> >
> > Commit a210fd32a46b ("kexec: add call to LSM hook in original
> > kexec_load syscall") introduced a new LSM hook. Assuming
> > CONFIG_KEXEC_VERIFY_SIG is enabled, with commit b5ca117365d9 ("ima:
> > prevent kexec_load syscall based on runtime secureboot flag") we can
> > now block the kexec_load syscall. Without being able to block the
> > kexec_load syscall, verifying the kexec image signature via the
> > kexec_file_load syscall is kind of pointless.
> >
> > Unless you're planning on writing an LSM to prevent the kexec_load
> > syscall, I assume you'll want to enable integrity anyway.
>
> User can disable kexec_load in kernel config, and only allow
> kexec_file_load. But yes, this can be improved separately in case no
> IMA enabled.
>
> For the time being we can leave with it and fix like this series do.
>
> >
> > Mimi
> >
>
> Thanks
> Dave
Yes, for now, I think it's good to fix the problem by following this
patch series and get kexec_file_load work with platform keyring first.
Will adopt suggestion from Mimi in the previous reply and update the
patch series.
For other remaining potential issues, kexec_load not being protected,
it could be disabled by config, and the improvement may require more
discussion. And issues like where the keyring is located, dependency
to making the keyring available for more general use could be
discussed later.
--
Best Regards,
Kairui Song
Re: [RFC PATCH 2/2] kexec, KEYS: Make use of platform keyring for signature verify
Hi, Mimi, Dave I checked the previous patches: https://www.spinics.net/lists/keyrings/msg03518.html https://www.spinics.net/lists/keyrings/msg03517.html https://www.spinics.net/lists/keyrings/msg03516.html That the latest patched I could found that placed the platform keyring in certs/ However it didn't cc kexec list, and so I think Dave didn't receive them. I could compose a patch to use the previous design, how do you think? On Sun, Jan 13, 2019 at 9:40 AM Dave Young wrote: > > Hi, > > On 01/11/19 at 11:13am, Mimi Zohar wrote: > > On Fri, 2019-01-11 at 21:43 +0800, Dave Young wrote: > > [snip] > > > > > Personally I would like to see platform key separated from integrity. > > > But for the kexec_file part I think it is good at least it works with > > > this fix. > > > > > > Acked-by: Dave Young > > > > The original "platform" keyring patches that Nayna posted multiple > > times were in the certs directory, but nobody commented/responded. So > > she reworked the patches, moving them to the integrity directory and > > posted them (cc'ing the kexec mailing list). It's a bit late to be > > asking to move it, isn't it? > > Hmm, apologize for being late, I did not get chance to have a look the > old series. Since we have the needs now, it should be still fine > > Maybe Kairui can check Nayna's old series, see if he can do something > again? > > > > > Mimi > > > > Thanks > Dave -- Best Regards, Kairui Song
[RFC PATCH 1/2] integrity, KEYS: add a reference to platform keyring
Currently kexec_file_load will verify the kernel image being loaded
against .builtin_trusted_keys or .secondary_trusted_keys, but the
image could be signed with third part keys which will be provided by
platform or firmware and the keys won't be available in keyrings mentioned
above.
After commit ea93102f3224 ('integrity: Define a trusted platform keyring')
a .platform keyring is introduced to store the keys provided by platform
or firmware. And with a few following commits, now keys required to verify
the image is being imported to .platform keyring, but currently, only
IMA-appraisal could use the keyring and verify the image.
This patch exposes the .platform and makes other components, like
kexec_file_load, could use this .platform keyring to verify the
kernel image.
Suggested-by: Mimi Zohar
Signed-off-by: Kairui Song
---
certs/system_keyring.c| 3 +++
include/keys/system_keyring.h | 5 +
security/integrity/digsig.c | 4
3 files changed, 12 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 81728717523d..a61b95390b80 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -24,6 +24,9 @@ static struct key *builtin_trusted_keys;
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
static struct key *secondary_trusted_keys;
#endif
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+struct key *platform_trusted_keys;
+#endif
extern __initconst const u8 system_certificate_list[];
extern __initconst const unsigned long system_certificate_list_size;
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 359c2f936004..9eaf01d01036 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -61,5 +61,10 @@ static inline struct key *get_ima_blacklist_keyring(void)
}
#endif /* CONFIG_IMA_BLACKLIST_KEYRING */
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+
+extern struct key *platform_trusted_keys;
+
+#endif /* CONFIG_INTEGRITY_PLATFORM_KEYRING */
#endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index f45d6edecf99..26206240388d 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -89,6 +89,10 @@ static int __integrity_init_keyring(const unsigned int id,
key_perm_t perm,
keyring[id] = NULL;
}
+ if (id == INTEGRITY_KEYRING_PLATFORM) {
+ platform_trusted_keys = keyring[id];
+ }
+
return err;
}
--
2.20.1
[RFC PATCH 0/2] let kexec_file_load use platform keyring to verify the kernel image
Hi, This is a different approach for the previous patch: [RFC PATCH 0/1] KEYS, integrity: Link .platform keyring to .secondary_trusted_keys make kexec_file_load be able to verify the kernel image against keys provided by platform or firmware. This patch adds a .platform_trusted_keys in system_keyring as the reference to .platform keyring in integrity subsystem, when platform keyring is being initialized it will be updated. Another thing on my mind is that now kexec_file_load will still relay on CONFIG_INTEGRITY_PLATFORM_KEYRING and all its dependencies to be enabled to be able to verify the image against firmware keys. I'm thinking about to have something like CONFIG_PLATFORM_KEYRING and make the .platform keyring could be enabled for a more wider usage. Not sure if it's a good idea though. Tested in a VM with locally signed kernel with pesign and imported the cert to EFI's MokList variable. Kairui Song (2): integrity, KEYS: add a reference to platform keyring kexec, KEYS: Make use of platform keyring for signature verify arch/x86/kernel/kexec-bzimage64.c | 13 ++--- certs/system_keyring.c| 10 +- include/keys/system_keyring.h | 5 + include/linux/verification.h | 1 + security/integrity/digsig.c | 4 5 files changed, 29 insertions(+), 4 deletions(-) -- 2.20.1
[RFC PATCH 2/2] kexec, KEYS: Make use of platform keyring for signature verify
kexec_file_load will need to verify the kernel signed with third part
keys, and the keys could be stored in firmware, then got loaded into
the .platform keyring. Now we have a .platform_trusted_keyring
as the reference to .platform keyring, this patch makes use if it and
allow kexec_file_load to verify the image against keys in .platform
keyring.
This commit adds a VERIFY_USE_PLATFORM_KEYRING similar to previous
VERIFY_USE_SECONDARY_KEYRING indicating that verify_pkcs7_signature
should verify the signature using platform keyring. Also, decrease
the error message log level when verification failed with -ENOKEY,
so that if called tried multiple time with different keyring it
won't generate extra noises.
Signed-off-by: Kairui Song
---
arch/x86/kernel/kexec-bzimage64.c | 13 ++---
certs/system_keyring.c| 7 ++-
include/linux/verification.h | 1 +
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/kexec-bzimage64.c
b/arch/x86/kernel/kexec-bzimage64.c
index 7d97e432cbbc..a8a5c1773ccc 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -534,9 +534,16 @@ static int bzImage64_cleanup(void *loader_data)
#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
{
- return verify_pefile_signature(kernel, kernel_len,
- VERIFY_USE_SECONDARY_KEYRING,
- VERIFYING_KEXEC_PE_SIGNATURE);
+ int ret;
+ ret = verify_pefile_signature(kernel, kernel_len,
+ VERIFY_USE_SECONDARY_KEYRING,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+ if (ret == -ENOKEY) {
+ ret = verify_pefile_signature(kernel, kernel_len,
+ VERIFY_USE_PLATFORM_KEYRING,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+ }
+ return ret;
}
#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index a61b95390b80..7514e69e719f 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
static struct key *builtin_trusted_keys;
@@ -239,12 +240,16 @@ int verify_pkcs7_signature(const void *data, size_t len,
trusted_keys = secondary_trusted_keys;
#else
trusted_keys = builtin_trusted_keys;
+#endif
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+ } else if (trusted_keys == VERIFY_USE_PLATFORM_KEYRING) {
+ trusted_keys = platform_trusted_keys;
#endif
}
ret = pkcs7_validate_trust(pkcs7, trusted_keys);
if (ret < 0) {
if (ret == -ENOKEY)
- pr_err("PKCS#7 signature not signed with a trusted
key\n");
+ pr_devel("PKCS#7 signature not signed with a trusted
key\n");
goto error;
}
diff --git a/include/linux/verification.h b/include/linux/verification.h
index cfa4730d607a..018fb5f13d44 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -17,6 +17,7 @@
* should be used.
*/
#define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
+#define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)
/*
* The use to which an asymmetric key is being put.
--
2.20.1
Re: [PATCH 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled
CCing more people On Wed, Jan 9, 2019 at 2:45 PM Kairui Song wrote: > > Currenly with "efi=noruntime" in kernel command line, calling > kexec_file_load will raise below problem: > > [ 97.967067] BUG: unable to handle kernel NULL pointer dereference at > > [ 97.967894] #PF error: [normal kernel read fault] > ... > [ 97.980456] Call Trace: > [ 97.980724] efi_runtime_map_copy+0x28/0x30 > [ 97.981267] bzImage64_load+0x688/0x872 > [ 97.981794] arch_kexec_kernel_image_load+0x6d/0x70 > [ 97.982441] kimage_file_alloc_init+0x13e/0x220 > [ 97.983035] __x64_sys_kexec_file_load+0x144/0x290 > [ 97.983586] do_syscall_64+0x55/0x1a0 > [ 97.983962] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > When efi runtime is not enabled, efi memmap is not mapped, so just skip > EFI info setup. > > Suggested-by: Dave Young > Signed-off-by: Kairui Song > --- > arch/x86/kernel/kexec-bzimage64.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kernel/kexec-bzimage64.c > b/arch/x86/kernel/kexec-bzimage64.c > index 0d5efa34f359..53917a3ebf94 100644 > --- a/arch/x86/kernel/kexec-bzimage64.c > +++ b/arch/x86/kernel/kexec-bzimage64.c > @@ -167,6 +167,9 @@ setup_efi_state(struct boot_params *params, unsigned long > params_load_addr, > struct efi_info *current_ei = _params.efi_info; > struct efi_info *ei = >efi_info; > > + if (!efi_enabled(EFI_RUNTIME_SERVICES)) > + return 0; > + > if (!current_ei->efi_memmap_size) > return 0; > > -- > 2.20.1 > -- Best Regards, Kairui Song
Re: [PATCH 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=oldmap
CCing more people
On Wed, Jan 9, 2019 at 2:47 PM Kairui Song wrote:
>
> When efi=noruntime or efi=oldmap is used, EFI services won't be available
> in the second kernel, therefore the second kernel will not be able to get
> the ACPI RSDP address from firmware by calling EFI services and won't
> boot. Previously we are expecting the user to set the acpi_rsdp=
> on kernel command line for second kernel as there was no way to pass RSDP
> address to second kernel.
>
> After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from
> boot params if available'), now it's possible to set a acpi_rsdp_addr
> parameter in the boot_params passed to second kernel, this commit make
> use of it, detect and set the RSDP address when it's required for second
> kernel to boot.
>
> Tested with an EFI enabled KVM VM with efi=noruntime.
>
> Suggested-by: Dave Young
> Signed-off-by: Kairui Song
> ---
> arch/x86/kernel/kexec-bzimage64.c | 21 +
> drivers/acpi/acpica/tbxfroot.c| 3 +--
> include/acpi/acpixf.h | 2 +-
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 53917a3ebf94..0a90dcbd041f 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct
> boot_params *params,
> /* Setup EFI state */
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> +
> +#ifdef CONFIG_ACPI
> + /* Setup ACPI RSDP pointer in case EFI is not available in second
> kernel */
> + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) ||
> efi_enabled(EFI_OLD_MEMMAP))) {
> + /* Copied from acpi_os_get_root_pointer accordingly */
> + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr;
> + if (!params->acpi_rsdp_addr) {
> + if (efi_enabled(EFI_CONFIG_TABLES)) {
> + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
> + params->acpi_rsdp_addr = efi.acpi20;
> + else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
> + params->acpi_rsdp_addr = efi.acpi;
> + } else if
> (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
> +
> acpi_find_root_pointer(>acpi_rsdp_addr);
> + }
> + }
> + if (!params->acpi_rsdp_addr)
> + pr_warn("RSDP is not available for second kernel\n");
> + }
> #endif
>
> +#endif
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
> diff --git a/drivers/acpi/acpica/tbxfroot.c b/drivers/acpi/acpica/tbxfroot.c
> index 483d0ce5180a..dac1e34a931c 100644
> --- a/drivers/acpi/acpica/tbxfroot.c
> +++ b/drivers/acpi/acpica/tbxfroot.c
> @@ -108,8 +108,7 @@ acpi_status acpi_tb_validate_rsdp(struct acpi_table_rsdp
> *rsdp)
> *
>
> **/
>
> -acpi_status ACPI_INIT_FUNCTION
> -acpi_find_root_pointer(acpi_physical_address *table_address)
> +acpi_status acpi_find_root_pointer(acpi_physical_address *table_address)
> {
> u8 *table_ptr;
> u8 *mem_rover;
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 7aa38b648564..869d75ecaf7d 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -474,7 +474,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
> ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
> acpi_reallocate_root_table(void))
>
> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
> +ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> acpi_find_root_pointer(acpi_physical_address
>*rsdp_address))
> ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> --
> 2.20.1
>
--
Best Regards,
Kairui Song
[PATCH 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=oldmap
When efi=noruntime or efi=oldmap is used, EFI services won't be available
in the second kernel, therefore the second kernel will not be able to get
the ACPI RSDP address from firmware by calling EFI services and won't
boot. Previously we are expecting the user to set the acpi_rsdp=
on kernel command line for second kernel as there was no way to pass RSDP
address to second kernel.
After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from
boot params if available'), now it's possible to set a acpi_rsdp_addr
parameter in the boot_params passed to second kernel, this commit make
use of it, detect and set the RSDP address when it's required for second
kernel to boot.
Tested with an EFI enabled KVM VM with efi=noruntime.
Suggested-by: Dave Young
Signed-off-by: Kairui Song
---
arch/x86/kernel/kexec-bzimage64.c | 21 +
drivers/acpi/acpica/tbxfroot.c| 3 +--
include/acpi/acpixf.h | 2 +-
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/kexec-bzimage64.c
b/arch/x86/kernel/kexec-bzimage64.c
index 53917a3ebf94..0a90dcbd041f 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct
boot_params *params,
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
+
+#ifdef CONFIG_ACPI
+ /* Setup ACPI RSDP pointer in case EFI is not available in second
kernel */
+ if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) ||
efi_enabled(EFI_OLD_MEMMAP))) {
+ /* Copied from acpi_os_get_root_pointer accordingly */
+ params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr;
+ if (!params->acpi_rsdp_addr) {
+ if (efi_enabled(EFI_CONFIG_TABLES)) {
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ params->acpi_rsdp_addr = efi.acpi20;
+ else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
+ params->acpi_rsdp_addr = efi.acpi;
+ } else if
(IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
+ acpi_find_root_pointer(>acpi_rsdp_addr);
+ }
+ }
+ if (!params->acpi_rsdp_addr)
+ pr_warn("RSDP is not available for second kernel\n");
+ }
#endif
+#endif
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
diff --git a/drivers/acpi/acpica/tbxfroot.c b/drivers/acpi/acpica/tbxfroot.c
index 483d0ce5180a..dac1e34a931c 100644
--- a/drivers/acpi/acpica/tbxfroot.c
+++ b/drivers/acpi/acpica/tbxfroot.c
@@ -108,8 +108,7 @@ acpi_status acpi_tb_validate_rsdp(struct acpi_table_rsdp
*rsdp)
*
**/
-acpi_status ACPI_INIT_FUNCTION
-acpi_find_root_pointer(acpi_physical_address *table_address)
+acpi_status acpi_find_root_pointer(acpi_physical_address *table_address)
{
u8 *table_ptr;
u8 *mem_rover;
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 7aa38b648564..869d75ecaf7d 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -474,7 +474,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
acpi_reallocate_root_table(void))
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
+ACPI_EXTERNAL_RETURN_STATUS(acpi_status
acpi_find_root_pointer(acpi_physical_address
*rsdp_address))
ACPI_EXTERNAL_RETURN_STATUS(acpi_status
--
2.20.1
[PATCH 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled
Currenly with "efi=noruntime" in kernel command line, calling kexec_file_load will raise below problem: [ 97.967067] BUG: unable to handle kernel NULL pointer dereference at [ 97.967894] #PF error: [normal kernel read fault] ... [ 97.980456] Call Trace: [ 97.980724] efi_runtime_map_copy+0x28/0x30 [ 97.981267] bzImage64_load+0x688/0x872 [ 97.981794] arch_kexec_kernel_image_load+0x6d/0x70 [ 97.982441] kimage_file_alloc_init+0x13e/0x220 [ 97.983035] __x64_sys_kexec_file_load+0x144/0x290 [ 97.983586] do_syscall_64+0x55/0x1a0 [ 97.983962] entry_SYSCALL_64_after_hwframe+0x44/0xa9 When efi runtime is not enabled, efi memmap is not mapped, so just skip EFI info setup. Suggested-by: Dave Young Signed-off-by: Kairui Song --- arch/x86/kernel/kexec-bzimage64.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 0d5efa34f359..53917a3ebf94 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -167,6 +167,9 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, struct efi_info *current_ei = _params.efi_info; struct efi_info *ei = >efi_info; + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return 0; + if (!current_ei->efi_memmap_size) return 0; -- 2.20.1
Re: [RFC PATCH 1/1] KEYS, integrity: Link .platform keyring to .secondary_trusted_keys
Thanks for the explanation Dave, my second thought is to let kexec use
the platform keyring directly, that is let kexec verify the image with
secondary/builtin keyring first then try platform keyring. And better
to make platform keyring independent of integrity subsystem, so kexec
could verify the image and don't depend on integrity. Any thought?
On Wed, Jan 9, 2019 at 9:34 AM Dave Young wrote:
>
> CC kexec list
> On 01/08/19 at 10:18am, Mimi Zohar wrote:
> > [Cc'ing the LSM and integrity mailing lists]
> >
> > Repeating my comment on PATCH 0/1 here with the expanded set of
> > mailing lists.
> >
> > The builtin and secondary keyrings have a signature change of trust
> > rooted in the signed kernel image. Adding the pre-boot keys to the
> > secondary keyring breaks that signature chain of trust.
> >
> > Please do NOT add the pre-boot "platform" keys to the secondary
> > keyring.
>
> If we regard kexec as a bootloader, it sounds natural to use the
> platform key to verify the signature with kexec_file_load syscall.
>
> It will be hard for user to manually sign a kernel and import the key
> then to reuse kexec_file_load.
>
> I think we do not care if platform key can be added to secondary or not,
> any suggestions how can kexec_file to use the platform key?
>
> >
> > Mimi
> >
> >
> > On Tue, 2019-01-08 at 16:12 +0800, Kairui Song wrote:
> > > Currently kexec may need to verify the kerne image, and the kernel image
> > > could be signed with third part keys which are provided by paltform or
> > > firmware (eg. stored in MokListRT EFI variable). And the same time,
> > > kexec_file_load will only verify the image agains .builtin_trusted_keys
> > > or .secondary_trusted_keys according to configuration, but there is no
> > > way for kexec_file_load to verify the image against any third part keys
> > > mentioned above.
> > >
> > > In ea93102f3224 ('integrity: Define a trusted platform keyring') a
> > > .platform keyring is introduced to store the keys provided by platform
> > > or firmware. And with a few following commits including 15ea0e1e3e185
> > > ('efi: Import certificates from UEFI Secure Boot'), now keys required to
> > > verify the image is being imported to .paltform keyring, and later
> > > IMA-appraisal could access the keyring and verify the image.
> > >
> > > This patch links the .platform keyring to .secondary_trusted_keys so
> > > kexec_file_load could also leverage the .platform keyring to verify the
> > > kernel image.
> > >
> > > Signed-off-by: Kairui Song
> > > ---
> > > certs/system_keyring.c | 30 ++
> > > include/keys/platform_keyring.h | 12
> > > security/integrity/digsig.c | 7 +++
> > > 3 files changed, 49 insertions(+)
> > > create mode 100644 include/keys/platform_keyring.h
> > >
> > > diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> > > index 81728717523d..dcef0259e149 100644
> > > --- a/certs/system_keyring.c
> > > +++ b/certs/system_keyring.c
> > > @@ -18,12 +18,14 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > >
> > > static struct key *builtin_trusted_keys;
> > > #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> > > static struct key *secondary_trusted_keys;
> > > #endif
> > > +static struct key *platform_keys = NULL;
> > >
> > > extern __initconst const u8 system_certificate_list[];
> > > extern __initconst const unsigned long system_certificate_list_size;
> > > @@ -67,6 +69,12 @@ int restrict_link_by_builtin_and_secondary_trusted(
> > > /* Allow the builtin keyring to be added to the secondary */
> > > return 0;
> > >
> > > + if (type == _type_keyring &&
> > > + dest_keyring == secondary_trusted_keys &&
> > > + payload == _keys->payload)
> > > + /* Allow the platform keyring to be added to the secondary */
> > > + return 0;
> > > +
> > > return restrict_link_by_signature(dest_keyring, type, payload,
> > > secondary_trusted_keys);
> > > }
> > > @@ -188,6 +196,28 @@ static __init int load_system_certificate_list(void)
> > > }
> > > late_initcall(load_system_certificate_list);
> > >
> > > +#if defined(CONFIG_INTEGRITY_PLATFORM_KEYRING
[RFC PATCH 1/1] KEYS, integrity: Link .platform keyring to .secondary_trusted_keys
Currently kexec may need to verify the kerne image, and the kernel image
could be signed with third part keys which are provided by paltform or
firmware (eg. stored in MokListRT EFI variable). And the same time,
kexec_file_load will only verify the image agains .builtin_trusted_keys
or .secondary_trusted_keys according to configuration, but there is no
way for kexec_file_load to verify the image against any third part keys
mentioned above.
In ea93102f3224 ('integrity: Define a trusted platform keyring') a
.platform keyring is introduced to store the keys provided by platform
or firmware. And with a few following commits including 15ea0e1e3e185
('efi: Import certificates from UEFI Secure Boot'), now keys required to
verify the image is being imported to .paltform keyring, and later
IMA-appraisal could access the keyring and verify the image.
This patch links the .platform keyring to .secondary_trusted_keys so
kexec_file_load could also leverage the .platform keyring to verify the
kernel image.
Signed-off-by: Kairui Song
---
certs/system_keyring.c | 30 ++
include/keys/platform_keyring.h | 12
security/integrity/digsig.c | 7 +++
3 files changed, 49 insertions(+)
create mode 100644 include/keys/platform_keyring.h
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 81728717523d..dcef0259e149 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -18,12 +18,14 @@
#include
#include
#include
+#include
#include
static struct key *builtin_trusted_keys;
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
static struct key *secondary_trusted_keys;
#endif
+static struct key *platform_keys = NULL;
extern __initconst const u8 system_certificate_list[];
extern __initconst const unsigned long system_certificate_list_size;
@@ -67,6 +69,12 @@ int restrict_link_by_builtin_and_secondary_trusted(
/* Allow the builtin keyring to be added to the secondary */
return 0;
+ if (type == _type_keyring &&
+ dest_keyring == secondary_trusted_keys &&
+ payload == _keys->payload)
+ /* Allow the platform keyring to be added to the secondary */
+ return 0;
+
return restrict_link_by_signature(dest_keyring, type, payload,
secondary_trusted_keys);
}
@@ -188,6 +196,28 @@ static __init int load_system_certificate_list(void)
}
late_initcall(load_system_certificate_list);
+#if defined(CONFIG_INTEGRITY_PLATFORM_KEYRING) &&
defined(CONFIG_SECONDARY_TRUSTED_KEYRING)
+
+/*
+ * Link .platform keyring to .secondary_trusted_key keyring
+ */
+static __init int load_platform_certificate_list(void)
+{
+ int ret = 0;
+ platform_keys = integrity_get_platform_keyring();
+ if (!platform_keys) {
+ return 0;
+ }
+ ret = key_link(secondary_trusted_keys, platform_keys);
+ if (ret < 0) {
+ pr_err("Failed to link platform keyring: %d", ret);
+ }
+ return 0;
+}
+late_initcall(load_platform_certificate_list);
+
+#endif
+
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
/**
diff --git a/include/keys/platform_keyring.h b/include/keys/platform_keyring.h
new file mode 100644
index ..4f92ed6c0b42
--- /dev/null
+++ b/include/keys/platform_keyring.h
@@ -0,0 +1,12 @@
+#ifndef _KEYS_PLATFORM_KEYRING_H
+#define _KEYS_PLATFORM_KEYRING_H
+
+#include
+
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+
+extern const struct key* __init integrity_get_platform_keyring(void);
+
+#endif /* CONFIG_INTEGRITY_PLATFORM_KEYRING */
+
+#endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index f45d6edecf99..397758d4f12d 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -176,3 +176,10 @@ int __init integrity_load_cert(const unsigned int id,
const char *source,
pr_info("Loading X.509 certificate: %s\n", source);
return integrity_add_key(id, data, len, perm);
}
+
+#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
+struct key* __init integrity_get_platform_keyring(void)
+{
+ return keyring[INTEGRITY_KEYRING_PLATFORM];
+}
+#endif
--
2.20.1
[RFC PATCH 0/1] KEYS, integrity: Link .platform keyring to .secondary_trusted_keys
Hi, as the subject, this is a patch that links the new introduced .platform keyring into .secondary_trusted_keys keyring. This is mainly for the kexec_file_load, make kexec_file_load be able to verify the kernel image agains keys provided by platform or firmware. kexec_file_load already could verify the image agains secondary_trusted_keys if secondary_trusted_keys exits, so this will make kexec_file_load be ware of platform keys as well. This may also useful for things like module sign verify that are using secondary_trusted_keys. I'm not sure if it will be better to move the INTEGRITY_PLATFORM_KEYRING to certs/ and let integrity subsystem use the keyring there, so just linked the .platform keyring into kernel's .secondary_trusted_keys keyring. It workd for my case, tested in a VM, I signed the kernel image locally with pesign and imported the cert to EFI's MokList variable. Kairui Song (1): KEYS, integrity: Link .platform keyring to .secondary_trusted_keys certs/system_keyring.c | 30 ++ include/keys/platform_keyring.h | 12 security/integrity/digsig.c | 7 +++ 3 files changed, 49 insertions(+) create mode 100644 include/keys/platform_keyring.h -- 2.20.1
[PATCH v2] x86/gart/kcore: Exclude GART aperture from kcore
On machines where the GART aperture is mapped over physical RAM,
/proc/kcore contains the GART aperture range and reading it may lead
to kernel panic.
In 'commit 2a3e83c6f96c ("x86/gart: Exclude GART aperture from vmcore")',
a special workaround is applied for vmcore to let /proc/vmcore return
zeroes when attempting to read the aperture region, as vmcore and kcore
have the same issue, and after 'commit 707d4eefbdb3 ("Revert "[PATCH]
Insert GART region into resource map"")', userspace tools can't detect and
exclude GART region.
This patch applies the same workaround for kcore. Let /proc/kcore return
zeroes too when trying to read the aperture region to fix the issue that
reading GART region may raise unexpected exceptions.
This applies to both first and second kernels as GART may get
initialized in the first and second kernels.
To get the same workaround work for kcore, this patch implement a hook
infrastructure for kcore which is same as the hook infrastructure for
vmcore introduced in 'commit 997c136f518c ("fs/proc/vmcore.c: add hook
to read_from_oldmem() to check for non-ram pages")'', and reuses the
checking function gart_oldmem_pfn_is_ram introduced in
'commit 2a3e83c6f96c ("x86/gart: Exclude GART aperture from vmcore"),'
as the hook function, but rename to gart_mem_pfn_is_ram as now it's
for a more generic use.
Suggested-by: Baoquan He
Signed-off-by: Kairui Song
---
Update from V1:
- Fix a complie error when CONFIG_PROC_KCORE is not set
arch/x86/kernel/aperture_64.c | 12 +---
fs/proc/kcore.c | 34 ++
include/linux/kcore.h | 3 +++
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 58176b56354e..c8a56f083419 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -14,6 +14,7 @@
#define pr_fmt(fmt) "AGP: " fmt
#include
+#include
#include
#include
#include
@@ -57,7 +58,7 @@ int fallback_aper_force __initdata;
int fix_aperture __initdata = 1;
-#ifdef CONFIG_PROC_VMCORE
+#if defined(CONFIG_PROC_VMCORE) || defined(CONFIG_PROC_KCORE)
/*
* If the first kernel maps the aperture over e820 RAM, the kdump kernel will
* use the same range because it will remain configured in the northbridge.
@@ -66,7 +67,7 @@ int fix_aperture __initdata = 1;
*/
static unsigned long aperture_pfn_start, aperture_page_count;
-static int gart_oldmem_pfn_is_ram(unsigned long pfn)
+static int gart_mem_pfn_is_ram(unsigned long pfn)
{
return likely((pfn < aperture_pfn_start) ||
(pfn >= aperture_pfn_start + aperture_page_count));
@@ -76,7 +77,12 @@ static void exclude_from_vmcore(u64 aper_base, u32
aper_order)
{
aperture_pfn_start = aper_base >> PAGE_SHIFT;
aperture_page_count = (32 * 1024 * 1024) << aper_order >> PAGE_SHIFT;
- WARN_ON(register_oldmem_pfn_is_ram(_oldmem_pfn_is_ram));
+#ifdef CONFIG_PROC_VMCORE
+ WARN_ON(register_oldmem_pfn_is_ram(_mem_pfn_is_ram));
+#endif
+#ifdef CONFIG_PROC_KCORE
+ WARN_ON(register_mem_pfn_is_ram(_mem_pfn_is_ram));
+#endif
}
#else
static void exclude_from_vmcore(u64 aper_base, u32 aper_order)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index bbcc185062bb..24e13fcc31ea 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -54,6 +54,35 @@ static LIST_HEAD(kclist_head);
static DECLARE_RWSEM(kclist_lock);
static int kcore_need_update = 1;
+static int (*mem_pfn_is_ram)(unsigned long pfn);
+
+int register_mem_pfn_is_ram(int (*fn)(unsigned long pfn))
+{
+ if (mem_pfn_is_ram)
+ return -EBUSY;
+ mem_pfn_is_ram = fn;
+ return 0;
+}
+
+void unregister_mem_pfn_is_ram(void)
+{
+ mem_pfn_is_ram = NULL;
+ wmb();
+}
+
+static int pfn_is_ram(unsigned long pfn)
+{
+ int (*fn)(unsigned long pfn);
+ /* pfn is ram unless fn() checks pagetype */
+ int ret = 1;
+
+ fn = mem_pfn_is_ram;
+ if (fn)
+ ret = fn(pfn);
+
+ return ret;
+}
+
/* This doesn't grab kclist_lock, so it should only be used at init time. */
void __init kclist_add(struct kcore_list *new, void *addr, size_t size,
int type)
@@ -465,6 +494,11 @@ read_kcore(struct file *file, char __user *buffer, size_t
buflen, loff_t *fpos)
goto out;
}
m = NULL; /* skip the list anchor */
+ } else if (!pfn_is_ram(__pa(start) >> PAGE_SHIFT)) {
+ if (clear_user(buffer, tsz)) {
+ ret = -EFAULT;
+ goto out;
+ }
} else if (m->type == KCORE_VMALLOC) {
vread(buf, (char *)start, tsz);
/* we have to zero-fill user buffer eve
[PATCH 1/1] x86/gart/kcore: Exclude GART aperture from kcore
On machines where the GART aperture is mapped over physical RAM,
/proc/kcore contains the GART aperture range and reading it may lead
to kernel panic.
In 'commit 2a3e83c6f96c ("x86/gart: Exclude GART aperture from vmcore")',
a special workaround is applied for vmcore to let /proc/vmcore return
zeroes when attempting to read the aperture region, as vmcore and kcore
have the same issue, and after 'commit 707d4eefbdb3 ("Revert "[PATCH]
Insert GART region into resource map"")', userspace tools can't detect and
exclude GART region.
This patch applies the same workaround for kcore. Let /proc/kcore return
zeroes too when trying to read the aperture region to fix the issue that
reading GART region may raise unexpected exceptions.
This applies to both first and second kernels as GART may get
initialized in the first and second kernels.
To get the same workaround work for kcore, this patch implement a hook
infrastructure for kcore which is same as the hook infrastructure for
vmcore introduced in 'commit 997c136f518c ("fs/proc/vmcore.c: add hook
to read_from_oldmem() to check for non-ram pages")'', and reuses the
checking function gart_oldmem_pfn_is_ram introduced in
'commit 2a3e83c6f96c ("x86/gart: Exclude GART aperture from vmcore"),'
as the hook function, but rename to gart_mem_pfn_is_ram as now it's
for a more generic use.
Suggested-by: Baoquan He
Signed-off-by: Kairui Song
---
arch/x86/kernel/aperture_64.c | 6 --
fs/proc/kcore.c | 34 ++
include/linux/kcore.h | 3 +++
3 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 2c4d5ece7456..e2e45ae63309 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -14,6 +14,7 @@
#define pr_fmt(fmt) "AGP: " fmt
#include
+#include
#include
#include
#include
@@ -66,7 +67,7 @@ int fix_aperture __initdata = 1;
*/
static unsigned long aperture_pfn_start, aperture_page_count;
-static int gart_oldmem_pfn_is_ram(unsigned long pfn)
+static int gart_mem_pfn_is_ram(unsigned long pfn)
{
return likely((pfn < aperture_pfn_start) ||
(pfn >= aperture_pfn_start + aperture_page_count));
@@ -76,7 +77,8 @@ static void exclude_from_vmcore(u64 aper_base, u32 aper_order)
{
aperture_pfn_start = aper_base >> PAGE_SHIFT;
aperture_page_count = (32 * 1024 * 1024) << aper_order >> PAGE_SHIFT;
- WARN_ON(register_oldmem_pfn_is_ram(_oldmem_pfn_is_ram));
+ WARN_ON(register_oldmem_pfn_is_ram(_mem_pfn_is_ram));
+ WARN_ON(register_mem_pfn_is_ram(_mem_pfn_is_ram));
}
#else
static void exclude_from_vmcore(u64 aper_base, u32 aper_order)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index bbcc185062bb..24e13fcc31ea 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -54,6 +54,35 @@ static LIST_HEAD(kclist_head);
static DECLARE_RWSEM(kclist_lock);
static int kcore_need_update = 1;
+static int (*mem_pfn_is_ram)(unsigned long pfn);
+
+int register_mem_pfn_is_ram(int (*fn)(unsigned long pfn))
+{
+ if (mem_pfn_is_ram)
+ return -EBUSY;
+ mem_pfn_is_ram = fn;
+ return 0;
+}
+
+void unregister_mem_pfn_is_ram(void)
+{
+ mem_pfn_is_ram = NULL;
+ wmb();
+}
+
+static int pfn_is_ram(unsigned long pfn)
+{
+ int (*fn)(unsigned long pfn);
+ /* pfn is ram unless fn() checks pagetype */
+ int ret = 1;
+
+ fn = mem_pfn_is_ram;
+ if (fn)
+ ret = fn(pfn);
+
+ return ret;
+}
+
/* This doesn't grab kclist_lock, so it should only be used at init time. */
void __init kclist_add(struct kcore_list *new, void *addr, size_t size,
int type)
@@ -465,6 +494,11 @@ read_kcore(struct file *file, char __user *buffer, size_t
buflen, loff_t *fpos)
goto out;
}
m = NULL; /* skip the list anchor */
+ } else if (!pfn_is_ram(__pa(start) >> PAGE_SHIFT)) {
+ if (clear_user(buffer, tsz)) {
+ ret = -EFAULT;
+ goto out;
+ }
} else if (m->type == KCORE_VMALLOC) {
vread(buf, (char *)start, tsz);
/* we have to zero-fill user buffer even if no read */
diff --git a/include/linux/kcore.h b/include/linux/kcore.h
index 8c3f8c14eeaa..6818156fe520 100644
--- a/include/linux/kcore.h
+++ b/include/linux/kcore.h
@@ -56,4 +56,7 @@ void kclist_add_remap(struct kcore_list *m, void *addr, void
*vaddr, size_t sz)
}
#endif
+extern int register_mem_pfn_is_ram(int (*fn)(unsigned long pfn));
+extern void unregister_mem_pfn_is_ram(void);
+
#endif /* _LINUX_KCORE_H */
--
2.19.1
[tip:x86/urgent] x86/boot: Fix kexec booting failure in the SEV bit detection code
Commit-ID: bdec8d7fa55e6f5314ed72e5a0b435d90ff90548
Gitweb: https://git.kernel.org/tip/bdec8d7fa55e6f5314ed72e5a0b435d90ff90548
Author: Kairui Song
AuthorDate: Thu, 27 Sep 2018 20:38:45 +0800
Committer: Borislav Petkov
CommitDate: Thu, 27 Sep 2018 19:35:03 +0200
x86/boot: Fix kexec booting failure in the SEV bit detection code
Commit
1958b5fc4010 ("x86/boot: Add early boot support when running with SEV active")
can occasionally cause system resets when kexec-ing a second kernel even
if SEV is not active.
That's because get_sev_encryption_bit() uses 32-bit rIP-relative
addressing to read the value of enc_bit - a variable which caches a
previously detected encryption bit position - but kexec may allocate
the early boot code to a higher location, beyond the 32-bit addressing
limit.
In this case, garbage will be read and get_sev_encryption_bit() will
return the wrong value, leading to accessing memory with the wrong
encryption setting.
Therefore, remove enc_bit, and thus get rid of the need to do 32-bit
rIP-relative addressing in the first place.
[ bp: massage commit message heavily. ]
Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV
active")
Suggested-by: Borislav Petkov
Signed-off-by: Kairui Song
Signed-off-by: Borislav Petkov
Reviewed-by: Tom Lendacky
Cc: linux-kernel@vger.kernel.org
Cc: t...@linutronix.de
Cc: mi...@redhat.com
Cc: h...@zytor.com
Cc: brijesh.si...@amd.com
Cc: ke...@lists.infradead.org
Cc: dyo...@redhat.com
Cc: b...@redhat.com
Cc: gh...@redhat.com
Link: https://lkml.kernel.org/r/20180927123845.32052-1-kas...@redhat.com
---
arch/x86/boot/compressed/mem_encrypt.S | 19 ---
1 file changed, 19 deletions(-)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S
b/arch/x86/boot/compressed/mem_encrypt.S
index eaa843a52907..a480356e0ed8 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -25,20 +25,6 @@ ENTRY(get_sev_encryption_bit)
push%ebx
push%ecx
push%edx
- push%edi
-
- /*
-* RIP-relative addressing is needed to access the encryption bit
-* variable. Since we are running in 32-bit mode we need this call/pop
-* sequence to get the proper relative addressing.
-*/
- call1f
-1: popl%edi
- subl$1b, %edi
-
- movlenc_bit(%edi), %eax
- cmpl$0, %eax
- jge .Lsev_exit
/* Check if running under a hypervisor */
movl$1, %eax
@@ -69,15 +55,12 @@ ENTRY(get_sev_encryption_bit)
movl%ebx, %eax
andl$0x3f, %eax /* Return the encryption bit location */
- movl%eax, enc_bit(%edi)
jmp .Lsev_exit
.Lno_sev:
xor %eax, %eax
- movl%eax, enc_bit(%edi)
.Lsev_exit:
- pop %edi
pop %edx
pop %ecx
pop %ebx
@@ -113,8 +96,6 @@ ENTRY(set_sev_encryption_mask)
ENDPROC(set_sev_encryption_mask)
.data
-enc_bit:
- .int0x
#ifdef CONFIG_AMD_MEM_ENCRYPT
.balign 8
[tip:x86/urgent] x86/boot: Fix kexec booting failure in the SEV bit detection code
Commit-ID: bdec8d7fa55e6f5314ed72e5a0b435d90ff90548
Gitweb: https://git.kernel.org/tip/bdec8d7fa55e6f5314ed72e5a0b435d90ff90548
Author: Kairui Song
AuthorDate: Thu, 27 Sep 2018 20:38:45 +0800
Committer: Borislav Petkov
CommitDate: Thu, 27 Sep 2018 19:35:03 +0200
x86/boot: Fix kexec booting failure in the SEV bit detection code
Commit
1958b5fc4010 ("x86/boot: Add early boot support when running with SEV active")
can occasionally cause system resets when kexec-ing a second kernel even
if SEV is not active.
That's because get_sev_encryption_bit() uses 32-bit rIP-relative
addressing to read the value of enc_bit - a variable which caches a
previously detected encryption bit position - but kexec may allocate
the early boot code to a higher location, beyond the 32-bit addressing
limit.
In this case, garbage will be read and get_sev_encryption_bit() will
return the wrong value, leading to accessing memory with the wrong
encryption setting.
Therefore, remove enc_bit, and thus get rid of the need to do 32-bit
rIP-relative addressing in the first place.
[ bp: massage commit message heavily. ]
Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV
active")
Suggested-by: Borislav Petkov
Signed-off-by: Kairui Song
Signed-off-by: Borislav Petkov
Reviewed-by: Tom Lendacky
Cc: linux-kernel@vger.kernel.org
Cc: t...@linutronix.de
Cc: mi...@redhat.com
Cc: h...@zytor.com
Cc: brijesh.si...@amd.com
Cc: ke...@lists.infradead.org
Cc: dyo...@redhat.com
Cc: b...@redhat.com
Cc: gh...@redhat.com
Link: https://lkml.kernel.org/r/20180927123845.32052-1-kas...@redhat.com
---
arch/x86/boot/compressed/mem_encrypt.S | 19 ---
1 file changed, 19 deletions(-)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S
b/arch/x86/boot/compressed/mem_encrypt.S
index eaa843a52907..a480356e0ed8 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -25,20 +25,6 @@ ENTRY(get_sev_encryption_bit)
push%ebx
push%ecx
push%edx
- push%edi
-
- /*
-* RIP-relative addressing is needed to access the encryption bit
-* variable. Since we are running in 32-bit mode we need this call/pop
-* sequence to get the proper relative addressing.
-*/
- call1f
-1: popl%edi
- subl$1b, %edi
-
- movlenc_bit(%edi), %eax
- cmpl$0, %eax
- jge .Lsev_exit
/* Check if running under a hypervisor */
movl$1, %eax
@@ -69,15 +55,12 @@ ENTRY(get_sev_encryption_bit)
movl%ebx, %eax
andl$0x3f, %eax /* Return the encryption bit location */
- movl%eax, enc_bit(%edi)
jmp .Lsev_exit
.Lno_sev:
xor %eax, %eax
- movl%eax, enc_bit(%edi)
.Lsev_exit:
- pop %edi
pop %edx
pop %ecx
pop %ebx
@@ -113,8 +96,6 @@ ENTRY(set_sev_encryption_mask)
ENDPROC(set_sev_encryption_mask)
.data
-enc_bit:
- .int0x
#ifdef CONFIG_AMD_MEM_ENCRYPT
.balign 8
[PATCH] x86/boot: Fix kexec booting failure after SEV early boot support
Commit 1958b5fc4010 ("x86/boot: Add early boot support when running
with SEV active") is causing kexec becomes sometimes unstable even if
SEV is not active. kexec reboot won't start a second kernel bypassing
BIOS boot process, instead, the system got reset.
That's because, in get_sev_encryption_bit function, we are using
32-bit RIP-relative addressing to read the value of enc_bit, but
kexec may alloc the early boot up code to a higher location, which
is beyond 32-bit addressing limit. Some garbage will be read and
get_sev_encryption_bit will return the wrong value, which leads to
wrong memory page flag.
This patch removes the use of enc_bit, as currently, enc_bit's only
purpose is to avoid duplicated encryption bit reading, but the overhead
of reading encryption bit is so tiny, so no need to cache that.
Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV
active")
Suggested-by: Borislav Petkov
Signed-off-by: Kairui Song
---
arch/x86/boot/compressed/mem_encrypt.S | 19 ---
1 file changed, 19 deletions(-)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S
b/arch/x86/boot/compressed/mem_encrypt.S
index eaa843a52907..a480356e0ed8 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -25,20 +25,6 @@ ENTRY(get_sev_encryption_bit)
push%ebx
push%ecx
push%edx
- push%edi
-
- /*
-* RIP-relative addressing is needed to access the encryption bit
-* variable. Since we are running in 32-bit mode we need this call/pop
-* sequence to get the proper relative addressing.
-*/
- call1f
-1: popl%edi
- subl$1b, %edi
-
- movlenc_bit(%edi), %eax
- cmpl$0, %eax
- jge .Lsev_exit
/* Check if running under a hypervisor */
movl$1, %eax
@@ -69,15 +55,12 @@ ENTRY(get_sev_encryption_bit)
movl%ebx, %eax
andl$0x3f, %eax /* Return the encryption bit location */
- movl%eax, enc_bit(%edi)
jmp .Lsev_exit
.Lno_sev:
xor %eax, %eax
- movl%eax, enc_bit(%edi)
.Lsev_exit:
- pop %edi
pop %edx
pop %ecx
pop %ebx
@@ -113,8 +96,6 @@ ENTRY(set_sev_encryption_mask)
ENDPROC(set_sev_encryption_mask)
.data
-enc_bit:
- .int0x
#ifdef CONFIG_AMD_MEM_ENCRYPT
.balign 8
--
2.17.1
[PATCH] x86/boot: Fix kexec booting failure after SEV early boot support
Commit 1958b5fc4010 ("x86/boot: Add early boot support when running
with SEV active") is causing kexec becomes sometimes unstable even if
SEV is not active. kexec reboot won't start a second kernel bypassing
BIOS boot process, instead, the system got reset.
That's because, in get_sev_encryption_bit function, we are using
32-bit RIP-relative addressing to read the value of enc_bit, but
kexec may alloc the early boot up code to a higher location, which
is beyond 32-bit addressing limit. Some garbage will be read and
get_sev_encryption_bit will return the wrong value, which leads to
wrong memory page flag.
This patch removes the use of enc_bit, as currently, enc_bit's only
purpose is to avoid duplicated encryption bit reading, but the overhead
of reading encryption bit is so tiny, so no need to cache that.
Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV
active")
Suggested-by: Borislav Petkov
Signed-off-by: Kairui Song
---
arch/x86/boot/compressed/mem_encrypt.S | 19 ---
1 file changed, 19 deletions(-)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S
b/arch/x86/boot/compressed/mem_encrypt.S
index eaa843a52907..a480356e0ed8 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -25,20 +25,6 @@ ENTRY(get_sev_encryption_bit)
push%ebx
push%ecx
push%edx
- push%edi
-
- /*
-* RIP-relative addressing is needed to access the encryption bit
-* variable. Since we are running in 32-bit mode we need this call/pop
-* sequence to get the proper relative addressing.
-*/
- call1f
-1: popl%edi
- subl$1b, %edi
-
- movlenc_bit(%edi), %eax
- cmpl$0, %eax
- jge .Lsev_exit
/* Check if running under a hypervisor */
movl$1, %eax
@@ -69,15 +55,12 @@ ENTRY(get_sev_encryption_bit)
movl%ebx, %eax
andl$0x3f, %eax /* Return the encryption bit location */
- movl%eax, enc_bit(%edi)
jmp .Lsev_exit
.Lno_sev:
xor %eax, %eax
- movl%eax, enc_bit(%edi)
.Lsev_exit:
- pop %edi
pop %edx
pop %ecx
pop %ebx
@@ -113,8 +96,6 @@ ENTRY(set_sev_encryption_mask)
ENDPROC(set_sev_encryption_mask)
.data
-enc_bit:
- .int0x
#ifdef CONFIG_AMD_MEM_ENCRYPT
.balign 8
--
2.17.1
