On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so the IMA infrastructure is extended to allow non device
tree platforms to provide a log buffer. x86 then passes the IMA buffer
to the new kernel via the setup_data mechanism.
Signed-off-by: Jonathan McDowell <[email protected]>
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 37 ++++++++++++++++-
arch/x86/kernel/setup.c | 26 ++++++++++++
include/linux/ima.h | 1 +
security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++-
7 files changed, 134 insertions(+), 5 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b0142e01002e..bde4959d9bdc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2017,6 +2017,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index b25d3f82c2f3..2f7b138a9388 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -10,6 +10,7 @@
#define SETUP_EFI 4
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
+#define SETUP_IMA 7
#define SETUP_INDIRECT (1<<31)
@@ -171,6 +172,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..07625da33075 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+#ifdef CONFIG_IMA_KEXEC
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
+ unsigned long setup_data_phys;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+}
+#endif /* CONFIG_IMA_KEXEC */
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
@@ -247,6 +273,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
#endif
+
+#ifdef CONFIG_IMA_KEXEC
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data));
+#endif
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -401,7 +434,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
sizeof(struct setup_data) +
- sizeof(struct efi_setup_data);
+ sizeof(struct efi_setup_data) +
+ sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c95b9ac5a457..8b0e7725f918 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -335,6 +336,28 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+#ifdef CONFIG_IMA_KEXEC
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ memblock_reserve(data->addr, data->size);
+ ima_set_kexec_buffer(data->addr, data->size);
+ early_memunmap(data, sizeof(*data));
+}
+#else
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +383,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..f58aed7acad4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {}
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
+extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size);
#endif
#else
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419c50cfe6b9 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -10,6 +10,7 @@
#include <linux/seq_file.h>
#include <linux/vmalloc.h>
#include <linux/kexec.h>
+#include <linux/memblock.h>
#include <linux/of.h>
#include <linux/ima.h>
#include "ima.h"
@@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
}
#endif /* IMA_KEXEC */
+#ifndef CONFIG_OF
+static phys_addr_t ima_early_kexec_buffer_phys;
+static size_t ima_early_kexec_buffer_size;
+
+void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
+{
+ if (size == 0)
+ return;
+
+ ima_early_kexec_buffer_phys = phys_addr;
+ ima_early_kexec_buffer_size = size;
+}
+
+int __init ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+ return -ENOTSUPP;
+
+ if (ima_early_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_early_kexec_buffer_phys,
+ ima_early_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_early_kexec_buffer_phys = 0;
+ ima_early_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+ return -ENOTSUPP;
+
+ if (ima_early_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_early_kexec_buffer_phys);
+ *size = ima_early_kexec_buffer_size;
+
+ return 0;
+}
+
+#else
+
+void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
+{
+ pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
+}
+#endif /* CONFIG_OF */
+
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.34.1
Hi Jonathan,
On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so the IMA infrastructure is extended to allow non device
> tree platforms to provide a log buffer. x86 then passes the IMA buffer
> to the new kernel via the setup_data mechanism.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
FYI, after applying, building, and booting a kernel with this patch,
"kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline --
initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but
"kexec -s -e" fails to reboot, at least on a test laptop even with only
the "boot_aggregate" measurement record.
Without enabling CONFIG_IMA_KEXEC, kexec boots properly.
thanks,
Mimi
> ---
> arch/x86/Kconfig | 1 +
> arch/x86/include/uapi/asm/bootparam.h | 9 ++++
> arch/x86/kernel/e820.c | 6 +--
> arch/x86/kernel/kexec-bzimage64.c | 37 ++++++++++++++++-
> arch/x86/kernel/setup.c | 26 ++++++++++++
> include/linux/ima.h | 1 +
> security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++-
> 7 files changed, 134 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b0142e01002e..bde4959d9bdc 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2017,6 +2017,7 @@ config KEXEC_FILE
> bool "kexec file based system call"
> select KEXEC_CORE
> select BUILD_BIN2C
> + select HAVE_IMA_KEXEC if IMA
> depends on X86_64
> depends on CRYPTO=y
> depends on CRYPTO_SHA256=y
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index b25d3f82c2f3..2f7b138a9388 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -10,6 +10,7 @@
> #define SETUP_EFI 4
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
> +#define SETUP_IMA 7
>
> #define SETUP_INDIRECT (1<<31)
>
> @@ -171,6 +172,14 @@ struct jailhouse_setup_data {
> } __attribute__((packed)) v2;
> } __attribute__((packed));
>
> +/*
> + * IMA buffer setup data information from the previous kernel during kexec
> + */
> +struct ima_setup_data {
> + __u64 addr;
> + __u64 size;
> +} __attribute__((packed));
> +
> /* The so-called "zeropage" */
> struct boot_params {
> struct screen_info screen_info; /* 0x000 */
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index f267205f2d5a..9dac24680ff8 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
> e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
>
> /*
> - * SETUP_EFI is supplied by kexec and does not need to be
> - * reserved.
> + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
> + * to be reserved.
> */
> - if (data->type != SETUP_EFI)
> + if (data->type != SETUP_EFI && data->type != SETUP_IMA)
> e820__range_update_kexec(pa_data,
> sizeof(*data) + data->len,
> E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..07625da33075 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> }
> #endif /* CONFIG_EFI */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void
> +setup_ima_state(const struct kimage *image, struct boot_params *params,
> + unsigned long params_load_addr,
> + unsigned int ima_setup_data_offset)
> +{
> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> + struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
> + unsigned long setup_data_phys;
> +
> + if (!image->ima_buffer_size)
> + return;
> +
> + sd->type = SETUP_IMA;
> + sd->len = sizeof(*ima);
> +
> + ima->addr = image->ima_buffer_addr;
> + ima->size = image->ima_buffer_size;
> +
> + /* Add setup data */
> + setup_data_phys = params_load_addr + ima_setup_data_offset;
> + sd->next = params->hdr.setup_data;
> + params->hdr.setup_data = setup_data_phys;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> +
> static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> @@ -247,6 +273,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Setup IMA log buffer state */
> + setup_ima_state(image, params, params_load_addr,
> + efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data));
> +#endif
> +
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
> @@ -401,7 +434,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> sizeof(struct setup_data) +
> - sizeof(struct efi_setup_data);
> + sizeof(struct efi_setup_data) +
> + sizeof(struct setup_data) +
> + sizeof(struct ima_setup_data);
>
> params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> if (!params)
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index c95b9ac5a457..8b0e7725f918 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -11,6 +11,7 @@
> #include <linux/dma-map-ops.h>
> #include <linux/dmi.h>
> #include <linux/efi.h>
> +#include <linux/ima.h>
> #include <linux/init_ohci1394_dma.h>
> #include <linux/initrd.h>
> #include <linux/iscsi_ibft.h>
> @@ -335,6 +336,28 @@ static void __init reserve_initrd(void)
> }
> #endif /* CONFIG_BLK_DEV_INITRD */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + struct ima_setup_data *data;
> +
> + data = early_memremap(phys_addr + sizeof(struct setup_data),
> + sizeof(*data));
> + if (!data) {
> + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> + return;
> + }
> + memblock_reserve(data->addr, data->size);
> + ima_set_kexec_buffer(data->addr, data->size);
> + early_memunmap(data, sizeof(*data));
> +}
> +#else
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
> +}
> +#endif
> +
> static void __init parse_setup_data(void)
> {
> struct setup_data *data;
> @@ -360,6 +383,9 @@ static void __init parse_setup_data(void)
> case SETUP_EFI:
> parse_efi_setup(pa_data, data_len);
> break;
> + case SETUP_IMA:
> + add_early_ima_buffer(pa_data);
> + break;
> default:
> break;
> }
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 426b1744215e..f58aed7acad4 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {}
>
> #ifdef CONFIG_IMA_KEXEC
> extern void ima_add_kexec_buffer(struct kimage *image);
> +extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size);
> #endif
>
> #else
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 13753136f03f..419c50cfe6b9 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -10,6 +10,7 @@
> #include <linux/seq_file.h>
> #include <linux/vmalloc.h>
> #include <linux/kexec.h>
> +#include <linux/memblock.h>
> #include <linux/of.h>
> #include <linux/ima.h>
> #include "ima.h"
> @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> }
> #endif /* IMA_KEXEC */
>
> +#ifndef CONFIG_OF
> +static phys_addr_t ima_early_kexec_buffer_phys;
> +static size_t ima_early_kexec_buffer_size;
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + if (size == 0)
> + return;
> +
> + ima_early_kexec_buffer_phys = phys_addr;
> + ima_early_kexec_buffer_size = size;
> +}
> +
> +int __init ima_free_kexec_buffer(void)
> +{
> + int rc;
> +
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + rc = memblock_phys_free(ima_early_kexec_buffer_phys,
> + ima_early_kexec_buffer_size);
> + if (rc)
> + return rc;
> +
> + ima_early_kexec_buffer_phys = 0;
> + ima_early_kexec_buffer_size = 0;
> +
> + return 0;
> +}
> +
> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + *addr = __va(ima_early_kexec_buffer_phys);
> + *size = ima_early_kexec_buffer_size;
> +
> + return 0;
> +}
> +
> +#else
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
> +}
> +#endif /* CONFIG_OF */
> +
> /*
> * Restore the measurement list from the previous kernel.
> */
> -void ima_load_kexec_buffer(void)
> +void __init ima_load_kexec_buffer(void)
> {
> void *kexec_buffer = NULL;
> size_t kexec_buffer_size = 0;
On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote:
> On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote:
> > Hi Jonathan,
> >
> > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote:
> > > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > > may verify the IMA signature of the kernel and initramfs, and measure
> > > it. The command line parameters passed to the kernel in the kexec call
> > > may also be measured by IMA. A remote attestation service can verify
> > > a TPM quote based on the TPM event log, the IMA measurement list, and
> > > the TPM PCR data. This can be achieved only if the IMA measurement log
> > > is carried over from the current kernel to the next kernel across
> > > the kexec call.
> > >
> > > powerpc and ARM64 both achieve this using device tree with a
> > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > > device tree, so the IMA infrastructure is extended to allow non device
> > > tree platforms to provide a log buffer. x86 then passes the IMA buffer
> > > to the new kernel via the setup_data mechanism.
> > >
> > > Signed-off-by: Jonathan McDowell <[email protected]>
> >
> > FYI, after applying, building, and booting a kernel with this patch,
> > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline --
> > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but
> > "kexec -s -e" fails to reboot, at least on a test laptop even with only
> > the "boot_aggregate" measurement record.
> >
> > Without enabling CONFIG_IMA_KEXEC, kexec boots properly.
>
> Thanks for giving it a try. At a guess your laptop is booting with
> EFI, whereas for my testing I was using qemu with legacy BIOS. I've
> managed to reproduce the issue with qemu+OVMF and isolated the mistake
> in the setup data calculation I made when EFI is involved. If you have
> time can you try with the below on top of the original patch?
Thank you! With the change, as expected there are two "boot_aggregate"
records in the measurement list. With a custom policy, the measurement
list verifies.
# grep boot_aggregate
/sys/kernel/security/ima/ascii_runtime_measurements
10 fe0b821290b1bd229e0d34c5571f48eeff403119 ima-sig
sha1:a87d47e560d148cd1f4c8da677a84ddbe27f12f8 boot_aggregate
10 fe0b821290b1bd229e0d34c5571f48eeff403119 ima-sig
sha1:a87d47e560d148cd1f4c8da677a84ddbe27f12f8 boot_aggregate
# cat /sys/kernel/security/ima/runtime_measurements_count
5597
# evmctl ima_measurement
/sys/kernel/security/ima/binary_runtime_measurements
Matched per TPM
bank calculated digest(s).
FYI, the builtin "ima_policy=tcb" results in measurement violations.
Normally, the measurement list can still be verified using the evmctl
"--ignore-violations" option. For some reason with the "tcb" policy,
the measurement list doesn't verify even with the "--ignore-violations"
option after kexec. I assume this is a result of additional
measurements being added after the kexec load, which aren't being
carried across kexec.
thanks,
Mimi
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 07625da33075..cdc73e081585 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -277,7 +277,9 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> #ifdef CONFIG_IMA_KEXEC
> /* Setup IMA log buffer state */
> setup_ima_state(image, params, params_load_addr,
> - efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data));
> + efi_setup_data_offset +
> + sizeof(struct setup_data) +
> + sizeof(struct efi_setup_data));
> #endif
>
> /* Setup EDD info */
On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote:
> Hi Jonathan,
>
> On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so the IMA infrastructure is extended to allow non device
> > tree platforms to provide a log buffer. x86 then passes the IMA buffer
> > to the new kernel via the setup_data mechanism.
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
>
> FYI, after applying, building, and booting a kernel with this patch,
> "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline --
> initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but
> "kexec -s -e" fails to reboot, at least on a test laptop even with only
> the "boot_aggregate" measurement record.
>
> Without enabling CONFIG_IMA_KEXEC, kexec boots properly.
Thanks for giving it a try. At a guess your laptop is booting with
EFI, whereas for my testing I was using qemu with legacy BIOS. I've
managed to reproduce the issue with qemu+OVMF and isolated the mistake
in the setup data calculation I made when EFI is involved. If you have
time can you try with the below on top of the original patch?
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 07625da33075..cdc73e081585 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -277,7 +277,9 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_IMA_KEXEC
/* Setup IMA log buffer state */
setup_ima_state(image, params, params_load_addr,
- efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data));
+ efi_setup_data_offset +
+ sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data));
#endif
/* Setup EDD info */
On Tue, Apr 26, 2022 at 09:49:53AM -0400, Mimi Zohar wrote:
> On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote:
> > On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote:
> > > Hi Jonathan,
> > >
> > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote:
> > > > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > > > may verify the IMA signature of the kernel and initramfs, and measure
> > > > it. The command line parameters passed to the kernel in the kexec call
> > > > may also be measured by IMA. A remote attestation service can verify
> > > > a TPM quote based on the TPM event log, the IMA measurement list, and
> > > > the TPM PCR data. This can be achieved only if the IMA measurement log
> > > > is carried over from the current kernel to the next kernel across
> > > > the kexec call.
> > > >
> > > > powerpc and ARM64 both achieve this using device tree with a
> > > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > > > device tree, so the IMA infrastructure is extended to allow non device
> > > > tree platforms to provide a log buffer. x86 then passes the IMA buffer
> > > > to the new kernel via the setup_data mechanism.
> > > >
> > > > Signed-off-by: Jonathan McDowell <[email protected]>
> > >
> > > FYI, after applying, building, and booting a kernel with this patch,
> > > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline --
> > > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but
> > > "kexec -s -e" fails to reboot, at least on a test laptop even with only
> > > the "boot_aggregate" measurement record.
> > >
> > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly.
> >
> > Thanks for giving it a try. At a guess your laptop is booting with
> > EFI, whereas for my testing I was using qemu with legacy BIOS. I've
> > managed to reproduce the issue with qemu+OVMF and isolated the mistake
> > in the setup data calculation I made when EFI is involved. If you have
> > time can you try with the below on top of the original patch?
>
> Thank you! With the change, as expected there are two "boot_aggregate"
> records in the measurement list. With a custom policy, the measurement
> list verifies.
Excellent, thanks for verifying. I'll get the fixed v2 out.
...
> FYI, the builtin "ima_policy=tcb" results in measurement violations.
> Normally, the measurement list can still be verified using the evmctl
> "--ignore-violations" option. For some reason with the "tcb" policy,
> the measurement list doesn't verify even with the "--ignore-violations"
> option after kexec. I assume this is a result of additional
> measurements being added after the kexec load, which aren't being
> carried across kexec.
I believe with "tcb" things like the subsequent exec of kexec to
actually do the reboot will end up measured, and as the kexec buffer is
static it won't include that.
Also there's an issue about the fact that we measure the kexec pieces
even if we don't actually do the kexec; there's no marker that confirms
the kexec took place. It's separate to this patch (in that it affects
the device tree kexec infrastructure too) but it's conceivable that an
attacker could measure in the new kernel details and not actually do the
kexec, and that's not distinguishable from the kexec happening.
One approach might be to add a marker in the kexec ima buffer such that
if it's not present we know the kexec hasn't happened, but I need to
think through that a bit more.
J.
On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote:
> On Tue, Apr 26, 2022 at 09:49:53AM -0400, Mimi Zohar wrote:
> > On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote:
> > > On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote:
> > > > Hi Jonathan,
> > > >
> > > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote:
> > > > > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > > > > may verify the IMA signature of the kernel and initramfs, and measure
> > > > > it. The command line parameters passed to the kernel in the kexec call
> > > > > may also be measured by IMA. A remote attestation service can verify
> > > > > a TPM quote based on the TPM event log, the IMA measurement list, and
> > > > > the TPM PCR data. This can be achieved only if the IMA measurement log
> > > > > is carried over from the current kernel to the next kernel across
> > > > > the kexec call.
> > > > >
> > > > > powerpc and ARM64 both achieve this using device tree with a
> > > > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > > > > device tree, so the IMA infrastructure is extended to allow non device
> > > > > tree platforms to provide a log buffer. x86 then passes the IMA buffer
> > > > > to the new kernel via the setup_data mechanism.
> > > > >
> > > > > Signed-off-by: Jonathan McDowell <[email protected]>
> > > >
> > > > FYI, after applying, building, and booting a kernel with this patch,
> > > > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline --
> > > > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but
> > > > "kexec -s -e" fails to reboot, at least on a test laptop even with only
> > > > the "boot_aggregate" measurement record.
> > > >
> > > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly.
> > >
> > > Thanks for giving it a try. At a guess your laptop is booting with
> > > EFI, whereas for my testing I was using qemu with legacy BIOS. I've
> > > managed to reproduce the issue with qemu+OVMF and isolated the mistake
> > > in the setup data calculation I made when EFI is involved. If you have
> > > time can you try with the below on top of the original patch?
> >
> > Thank you! With the change, as expected there are two "boot_aggregate"
> > records in the measurement list. With a custom policy, the measurement
> > list verifies.
>
> Excellent, thanks for verifying. I'll get the fixed v2 out.
>
> ...
> > FYI, the builtin "ima_policy=tcb" results in measurement violations.
> > Normally, the measurement list can still be verified using the evmctl
> > "--ignore-violations" option. For some reason with the "tcb" policy,
> > the measurement list doesn't verify even with the "--ignore-violations"
> > option after kexec. I assume this is a result of additional
> > measurements being added after the kexec load, which aren't being
> > carried across kexec.
>
> I believe with "tcb" things like the subsequent exec of kexec to
> actually do the reboot will end up measured, and as the kexec buffer is
> static it won't include that.
>
> Also there's an issue about the fact that we measure the kexec pieces
> even if we don't actually do the kexec; there's no marker that confirms
> the kexec took place. It's separate to this patch (in that it affects
> the device tree kexec infrastructure too) but it's conceivable that an
> attacker could measure in the new kernel details and not actually do the
> kexec, and that's not distinguishable from the kexec happening.
>
> One approach might be to add a marker in the kexec ima buffer such that
> if it's not present we know the kexec hasn't happened, but I need to
> think through that a bit more.
I'm not quite sure what you mean by "we measure the kexec pieces". The
kexec file load syscall calls kernel_read_file_from_fd() to read the
kernel image into a buffer. The measurement record included in the IMA
measurement list a hash of the buffer data, which is exactly the same
as the hash of the kernel image.
The kernel kexec self tests only do the kexec load, not the execute.
For each kexec execute you'll see an additional "boot_aggregate" record
in the IMA measurement list. At least for the moment I don't see a
need for additional marker.
thanks,
Mimi
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so the IMA infrastructure is extended to allow non device
tree platforms to provide a log buffer. x86 then passes the IMA buffer
to the new kernel via the setup_data mechanism.
Signed-off-by: Jonathan McDowell <[email protected]>
---
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 39 +++++++++++++++++-
arch/x86/kernel/setup.c | 26 ++++++++++++
include/linux/ima.h | 1 +
security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++-
7 files changed, 136 insertions(+), 5 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b0142e01002e..bde4959d9bdc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2017,6 +2017,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index b25d3f82c2f3..2f7b138a9388 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -10,6 +10,7 @@
#define SETUP_EFI 4
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
+#define SETUP_IMA 7
#define SETUP_INDIRECT (1<<31)
@@ -171,6 +172,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..cdc73e081585 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+#ifdef CONFIG_IMA_KEXEC
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
+ unsigned long setup_data_phys;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+}
+#endif /* CONFIG_IMA_KEXEC */
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
@@ -247,6 +273,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
#endif
+
+#ifdef CONFIG_IMA_KEXEC
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ efi_setup_data_offset +
+ sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data));
+#endif
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -401,7 +436,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
sizeof(struct setup_data) +
- sizeof(struct efi_setup_data);
+ sizeof(struct efi_setup_data) +
+ sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c95b9ac5a457..8b0e7725f918 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -335,6 +336,28 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+#ifdef CONFIG_IMA_KEXEC
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ memblock_reserve(data->addr, data->size);
+ ima_set_kexec_buffer(data->addr, data->size);
+ early_memunmap(data, sizeof(*data));
+}
+#else
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +383,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..f58aed7acad4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {}
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
+extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size);
#endif
#else
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419c50cfe6b9 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -10,6 +10,7 @@
#include <linux/seq_file.h>
#include <linux/vmalloc.h>
#include <linux/kexec.h>
+#include <linux/memblock.h>
#include <linux/of.h>
#include <linux/ima.h>
#include "ima.h"
@@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
}
#endif /* IMA_KEXEC */
+#ifndef CONFIG_OF
+static phys_addr_t ima_early_kexec_buffer_phys;
+static size_t ima_early_kexec_buffer_size;
+
+void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
+{
+ if (size == 0)
+ return;
+
+ ima_early_kexec_buffer_phys = phys_addr;
+ ima_early_kexec_buffer_size = size;
+}
+
+int __init ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+ return -ENOTSUPP;
+
+ if (ima_early_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_early_kexec_buffer_phys,
+ ima_early_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_early_kexec_buffer_phys = 0;
+ ima_early_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
+ return -ENOTSUPP;
+
+ if (ima_early_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_early_kexec_buffer_phys);
+ *size = ima_early_kexec_buffer_size;
+
+ return 0;
+}
+
+#else
+
+void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
+{
+ pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
+}
+#endif /* CONFIG_OF */
+
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.34.1
On Thu, 2022-04-28 at 10:40 +0000, Jonathan McDowell wrote:
> On Tue, Apr 26, 2022 at 02:10:58PM -0400, Mimi Zohar wrote:
> > On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote:
> > > Also there's an issue about the fact that we measure the kexec pieces
> > > even if we don't actually do the kexec; there's no marker that confirms
> > > the kexec took place. It's separate to this patch (in that it affects
> > > the device tree kexec infrastructure too) but it's conceivable that an
> > > attacker could measure in the new kernel details and not actually do the
> > > kexec, and that's not distinguishable from the kexec happening.
> > >
> > > One approach might be to add a marker in the kexec ima buffer such that
> > > if it's not present we know the kexec hasn't happened, but I need to
> > > think through that a bit more.
> >
> > I'm not quite sure what you mean by "we measure the kexec pieces". The
> > kexec file load syscall calls kernel_read_file_from_fd() to read the
> > kernel image into a buffer. The measurement record included in the IMA
> > measurement list a hash of the buffer data, which is exactly the same
> > as the hash of the kernel image.
> >
> > The kernel kexec self tests only do the kexec load, not the execute.
> > For each kexec execute you'll see an additional "boot_aggregate" record
> > in the IMA measurement list. At least for the moment I don't see a
> > need for additional marker.
>
> You're right, of course. I'd missed the fact we measure the
> boot_aggregate into IMA_MEASURE_PCR_IDX on boot, so although we'll
> update PCRs related to the kexec on load the IMA PCR won't get updated
> until we've actually done the reboot. So no need for anything extra.
To clarify, after the kexec load, the IMA measurement list contains the
kexec'ed kernel image measurement. The TPM was also extended with that
measurement. Nothing prevents verifying the IMA measurement list at
this point, before the kexec execute, though depending on policy it
might result in additional measurements.
The IMA "pcr=" policy rule option allows specifying a different PCR
than the default IMA_MEASURE_PCR_IDX.
To summarize, with CONFIG_IMA_ARCH_POLICY enabled, both measurements -
kexec'ed kernel image, boot_aggregate - are being added to the IMA
measurement list and extended into the default TPM PCR. Measuring the
kexec'ed kernel image and extending the TPM with the measurement
happens at some point before the system is rebooted. The
"boot_aggregate" is the first measurement after boot/soft boot.
thanks,
Mimi
On Tue, Apr 26, 2022 at 02:10:58PM -0400, Mimi Zohar wrote:
> On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote:
> > Also there's an issue about the fact that we measure the kexec pieces
> > even if we don't actually do the kexec; there's no marker that confirms
> > the kexec took place. It's separate to this patch (in that it affects
> > the device tree kexec infrastructure too) but it's conceivable that an
> > attacker could measure in the new kernel details and not actually do the
> > kexec, and that's not distinguishable from the kexec happening.
> >
> > One approach might be to add a marker in the kexec ima buffer such that
> > if it's not present we know the kexec hasn't happened, but I need to
> > think through that a bit more.
>
> I'm not quite sure what you mean by "we measure the kexec pieces". The
> kexec file load syscall calls kernel_read_file_from_fd() to read the
> kernel image into a buffer. The measurement record included in the IMA
> measurement list a hash of the buffer data, which is exactly the same
> as the hash of the kernel image.
>
> The kernel kexec self tests only do the kexec load, not the execute.
> For each kexec execute you'll see an additional "boot_aggregate" record
> in the IMA measurement list. At least for the moment I don't see a
> need for additional marker.
You're right, of course. I'd missed the fact we measure the
boot_aggregate into IMA_MEASURE_PCR_IDX on boot, so although we'll
update PCRs related to the kexec on load the IMA PCR won't get updated
until we've actually done the reboot. So no need for anything extra.
J.
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 13753136f03f..419c50cfe6b9 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -10,6 +10,7 @@
> #include <linux/seq_file.h>
> #include <linux/vmalloc.h>
> #include <linux/kexec.h>
> +#include <linux/memblock.h>
> #include <linux/of.h>
> #include <linux/ima.h>
> #include "ima.h"
> @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> }
> #endif /* IMA_KEXEC */
>
> +#ifndef CONFIG_OF
> +static phys_addr_t ima_early_kexec_buffer_phys;
> +static size_t ima_early_kexec_buffer_size;
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + if (size == 0)
> + return;
> +
> + ima_early_kexec_buffer_phys = phys_addr;
> + ima_early_kexec_buffer_size = size;
> +}
> +
> +int __init ima_free_kexec_buffer(void)
> +{
> + int rc;
> +
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + rc = memblock_phys_free(ima_early_kexec_buffer_phys,
> + ima_early_kexec_buffer_size);
> + if (rc)
> + return rc;
> +
> + ima_early_kexec_buffer_phys = 0;
> + ima_early_kexec_buffer_size = 0;
> +
> + return 0;
> +}
> +
> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + *addr = __va(ima_early_kexec_buffer_phys);
> + *size = ima_early_kexec_buffer_size;
> +
> + return 0;
> +}
> +
Originally both ima_get_kexec_buffer() and ima_free_kexec_buffer() were
architecture specific. Refer to commit 467d27824920 ("powerpc: ima:
get the kexec buffer passed by the previous kernel"). Is there any
need for defining them here behind an "#ifndef CONFIG_OF"?
> +#else
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
> +}
> +#endif /* CONFIG_OF */
> +
Only when "HAVE_IMA_KEXEC" is defined is this file included. Why is
this warning needed?
thanks,
Mimi
> /*
> * Restore the measurement list from the previous kernel.
> */
> -void ima_load_kexec_buffer(void)
> +void __init ima_load_kexec_buffer(void)
> {
> void *kexec_buffer = NULL;
> size_t kexec_buffer_size = 0;
On Fri, Apr 29, 2022 at 05:30:10PM -0400, Mimi Zohar wrote:
> > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > index 13753136f03f..419c50cfe6b9 100644
> > --- a/security/integrity/ima/ima_kexec.c
> > +++ b/security/integrity/ima/ima_kexec.c
> > @@ -10,6 +10,7 @@
> > #include <linux/seq_file.h>
> > #include <linux/vmalloc.h>
> > #include <linux/kexec.h>
> > +#include <linux/memblock.h>
> > #include <linux/of.h>
> > #include <linux/ima.h>
> > #include "ima.h"
> > @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> > }
> > #endif /* IMA_KEXEC */
> >
> > +#ifndef CONFIG_OF
> > +static phys_addr_t ima_early_kexec_buffer_phys;
> > +static size_t ima_early_kexec_buffer_size;
> > +
> > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> > +{
> > + if (size == 0)
> > + return;
> > +
> > + ima_early_kexec_buffer_phys = phys_addr;
> > + ima_early_kexec_buffer_size = size;
> > +}
> > +
> > +int __init ima_free_kexec_buffer(void)
> > +{
> > + int rc;
> > +
> > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> > + return -ENOTSUPP;
> > +
> > + if (ima_early_kexec_buffer_size == 0)
> > + return -ENOENT;
> > +
> > + rc = memblock_phys_free(ima_early_kexec_buffer_phys,
> > + ima_early_kexec_buffer_size);
> > + if (rc)
> > + return rc;
> > +
> > + ima_early_kexec_buffer_phys = 0;
> > + ima_early_kexec_buffer_size = 0;
> > +
> > + return 0;
> > +}
> > +
> > +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> > +{
> > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> > + return -ENOTSUPP;
> > +
> > + if (ima_early_kexec_buffer_size == 0)
> > + return -ENOENT;
> > +
> > + *addr = __va(ima_early_kexec_buffer_phys);
> > + *size = ima_early_kexec_buffer_size;
> > +
> > + return 0;
> > +}
> > +
>
> Originally both ima_get_kexec_buffer() and ima_free_kexec_buffer() were
> architecture specific. Refer to commit 467d27824920 ("powerpc: ima:
> get the kexec buffer passed by the previous kernel"). Is there any
> need for defining them here behind an "#ifndef CONFIG_OF"?
Commit fee3ff99bc67 (powerpc: Move arch independent ima kexec functions
to drivers/of/kexec.c) moved those functions to drivers/of/kexec.c as a
more generic implementation so that ARM64 could use them too.
I think for platforms that use device tree that's the way to go, but the
functions to generically set + get the IMA buffer for non device tree
systems were useful enough to put in the IMA code rather than being x86
specific. If you disagree I can move them under arch/x86/ (assuming the
x86 folk agree using setup_data is the right way to go, I haven't seen
any of them comment on this approach yet).
> > +#else
> > +
> > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> > +{
> > + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
> > +}
> > +#endif /* CONFIG_OF */
> > +
>
> Only when "HAVE_IMA_KEXEC" is defined is this file included. Why is
> this warning needed?
x86 *can* have device tree enabled, but the only platform I'm aware that
did it was OLPC and I haven't seen any of the distros enable it. I put
this in so there's a warning if we have CONFIG_OF enabled on x86 and
tried to pass the IMA log via setup_data. Can remove (or fold into the
x86 code if we go that way).
> > /*
> > * Restore the measurement list from the previous kernel.
> > */
> > -void ima_load_kexec_buffer(void)
> > +void __init ima_load_kexec_buffer(void)
> > {
> > void *kexec_buffer = NULL;
> > size_t kexec_buffer_size = 0;
J.
On Tue, 2022-05-03 at 12:02 +0000, Jonathan McDowell wrote:
> On Fri, Apr 29, 2022 at 05:30:10PM -0400, Mimi Zohar wrote:
> > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > > index 13753136f03f..419c50cfe6b9 100644
> > > --- a/security/integrity/ima/ima_kexec.c
> > > +++ b/security/integrity/ima/ima_kexec.c
> > > @@ -10,6 +10,7 @@
> > > #include <linux/seq_file.h>
> > > #include <linux/vmalloc.h>
> > > #include <linux/kexec.h>
> > > +#include <linux/memblock.h>
> > > #include <linux/of.h>
> > > #include <linux/ima.h>
> > > #include "ima.h"
> > > @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> > > }
> > > #endif /* IMA_KEXEC */
> > >
> > > +#ifndef CONFIG_OF
> > > +static phys_addr_t ima_early_kexec_buffer_phys;
> > > +static size_t ima_early_kexec_buffer_size;
> > > +
> > > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> > > +{
> > > + if (size == 0)
> > > + return;
> > > +
> > > + ima_early_kexec_buffer_phys = phys_addr;
> > > + ima_early_kexec_buffer_size = size;
> > > +}
> > > +
> > > +int __init ima_free_kexec_buffer(void)
> > > +{
> > > + int rc;
> > > +
> > > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> > > + return -ENOTSUPP;
> > > +
> > > + if (ima_early_kexec_buffer_size == 0)
> > > + return -ENOENT;
> > > +
> > > + rc = memblock_phys_free(ima_early_kexec_buffer_phys,
> > > + ima_early_kexec_buffer_size);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + ima_early_kexec_buffer_phys = 0;
> > > + ima_early_kexec_buffer_size = 0;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> > > +{
> > > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> > > + return -ENOTSUPP;
The Kconfig conditionally compiles ima_kexec.c based on
CONFIG_HAVE_IMA_KEXEC. This test should be removed from here and from
ima_get_kexec_buffer().
CONFIG_IMA_KEXEC controls whether or not to carry the measurement list
to the next kernel, not whether the measurement list should be
restored. Notice that ima_load_kexec_buffer() is not within the ifdef
CONFIG_IMA_KEXEC.
> > > +
> > > + if (ima_early_kexec_buffer_size == 0)
> > > + return -ENOENT;
There should always be at least one measurement - the boot_aggregate.
> > > +
> > > + *addr = __va(ima_early_kexec_buffer_phys);
> > > + *size = ima_early_kexec_buffer_size;
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > Originally both ima_get_kexec_buffer() and ima_free_kexec_buffer() were
> > architecture specific. Refer to commit 467d27824920 ("powerpc: ima:
> > get the kexec buffer passed by the previous kernel"). Is there any
> > need for defining them here behind an "#ifndef CONFIG_OF"?
>
> Commit fee3ff99bc67 (powerpc: Move arch independent ima kexec functions
> to drivers/of/kexec.c) moved those functions to drivers/of/kexec.c as a
> more generic implementation so that ARM64 could use them too.
>
> I think for platforms that use device tree that's the way to go, but the
> functions to generically set + get the IMA buffer for non device tree
> systems were useful enough to put in the IMA code rather than being x86
> specific. If you disagree I can move them under arch/x86/ (assuming the
> x86 folk agree using setup_data is the right way to go, I haven't seen
> any of them comment on this approach yet).
So other architectures will need to define CONFIG_HAVE_IMA_KEXEC, a
function to call ima_set_kexec_buffer() to restore the measurement
list, and a function equivalent to ima_setup_state().
After removing the unnecessary tests mentioned above, consider whether
there is still any benefit to defining these functions.
> > > +#else
> > > +
> > > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> > > +{
> > > + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
> > > +}
> > > +#endif /* CONFIG_OF */
> > > +
> >
> > Only when "HAVE_IMA_KEXEC" is defined is this file included. Why is
> > this warning needed?
>
> x86 *can* have device tree enabled, but the only platform I'm aware that
> did it was OLPC and I haven't seen any of the distros enable it. I put
> this in so there's a warning if we have CONFIG_OF enabled on x86 and
> tried to pass the IMA log via setup_data. Can remove (or fold into the
> x86 code if we go that way).
Thanks for the explanation.
> > > /*
> > > * Restore the measurement list from the previous kernel.
> > > */
> > > -void ima_load_kexec_buffer(void)
> > > +void __init ima_load_kexec_buffer(void)
> > > {
> > > void *kexec_buffer = NULL;
> > > size_t kexec_buffer_size = 0;
>
> J.
thanks,
Mimi
Can someone from the x86 side provide some feedback on whether using
setup_data to pass this data across the kexec boundary is appropriate,
and if not point me in a better direction?
On Tue, Apr 26, 2022 at 05:53:47PM +0100, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so the IMA infrastructure is extended to allow non device
> tree platforms to provide a log buffer. x86 then passes the IMA buffer
> to the new kernel via the setup_data mechanism.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> ---
> v2:
> - Fix operation with EFI systems
> ---
> arch/x86/Kconfig | 1 +
> arch/x86/include/uapi/asm/bootparam.h | 9 ++++
> arch/x86/kernel/e820.c | 6 +--
> arch/x86/kernel/kexec-bzimage64.c | 39 +++++++++++++++++-
> arch/x86/kernel/setup.c | 26 ++++++++++++
> include/linux/ima.h | 1 +
> security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++-
> 7 files changed, 136 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index b0142e01002e..bde4959d9bdc 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2017,6 +2017,7 @@ config KEXEC_FILE
> bool "kexec file based system call"
> select KEXEC_CORE
> select BUILD_BIN2C
> + select HAVE_IMA_KEXEC if IMA
> depends on X86_64
> depends on CRYPTO=y
> depends on CRYPTO_SHA256=y
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index b25d3f82c2f3..2f7b138a9388 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -10,6 +10,7 @@
> #define SETUP_EFI 4
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
> +#define SETUP_IMA 7
>
> #define SETUP_INDIRECT (1<<31)
>
> @@ -171,6 +172,14 @@ struct jailhouse_setup_data {
> } __attribute__((packed)) v2;
> } __attribute__((packed));
>
> +/*
> + * IMA buffer setup data information from the previous kernel during kexec
> + */
> +struct ima_setup_data {
> + __u64 addr;
> + __u64 size;
> +} __attribute__((packed));
> +
> /* The so-called "zeropage" */
> struct boot_params {
> struct screen_info screen_info; /* 0x000 */
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index f267205f2d5a..9dac24680ff8 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
> e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
>
> /*
> - * SETUP_EFI is supplied by kexec and does not need to be
> - * reserved.
> + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
> + * to be reserved.
> */
> - if (data->type != SETUP_EFI)
> + if (data->type != SETUP_EFI && data->type != SETUP_IMA)
> e820__range_update_kexec(pa_data,
> sizeof(*data) + data->len,
> E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..cdc73e081585 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> }
> #endif /* CONFIG_EFI */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void
> +setup_ima_state(const struct kimage *image, struct boot_params *params,
> + unsigned long params_load_addr,
> + unsigned int ima_setup_data_offset)
> +{
> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> + struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
> + unsigned long setup_data_phys;
> +
> + if (!image->ima_buffer_size)
> + return;
> +
> + sd->type = SETUP_IMA;
> + sd->len = sizeof(*ima);
> +
> + ima->addr = image->ima_buffer_addr;
> + ima->size = image->ima_buffer_size;
> +
> + /* Add setup data */
> + setup_data_phys = params_load_addr + ima_setup_data_offset;
> + sd->next = params->hdr.setup_data;
> + params->hdr.setup_data = setup_data_phys;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> +
> static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> @@ -247,6 +273,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Setup IMA log buffer state */
> + setup_ima_state(image, params, params_load_addr,
> + efi_setup_data_offset +
> + sizeof(struct setup_data) +
> + sizeof(struct efi_setup_data));
> +#endif
> +
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
> @@ -401,7 +436,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> sizeof(struct setup_data) +
> - sizeof(struct efi_setup_data);
> + sizeof(struct efi_setup_data) +
> + sizeof(struct setup_data) +
> + sizeof(struct ima_setup_data);
>
> params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> if (!params)
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index c95b9ac5a457..8b0e7725f918 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -11,6 +11,7 @@
> #include <linux/dma-map-ops.h>
> #include <linux/dmi.h>
> #include <linux/efi.h>
> +#include <linux/ima.h>
> #include <linux/init_ohci1394_dma.h>
> #include <linux/initrd.h>
> #include <linux/iscsi_ibft.h>
> @@ -335,6 +336,28 @@ static void __init reserve_initrd(void)
> }
> #endif /* CONFIG_BLK_DEV_INITRD */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + struct ima_setup_data *data;
> +
> + data = early_memremap(phys_addr + sizeof(struct setup_data),
> + sizeof(*data));
> + if (!data) {
> + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> + return;
> + }
> + memblock_reserve(data->addr, data->size);
> + ima_set_kexec_buffer(data->addr, data->size);
> + early_memunmap(data, sizeof(*data));
> +}
> +#else
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
> +}
> +#endif
> +
> static void __init parse_setup_data(void)
> {
> struct setup_data *data;
> @@ -360,6 +383,9 @@ static void __init parse_setup_data(void)
> case SETUP_EFI:
> parse_efi_setup(pa_data, data_len);
> break;
> + case SETUP_IMA:
> + add_early_ima_buffer(pa_data);
> + break;
> default:
> break;
> }
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 426b1744215e..f58aed7acad4 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {}
>
> #ifdef CONFIG_IMA_KEXEC
> extern void ima_add_kexec_buffer(struct kimage *image);
> +extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size);
> #endif
>
> #else
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 13753136f03f..419c50cfe6b9 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -10,6 +10,7 @@
> #include <linux/seq_file.h>
> #include <linux/vmalloc.h>
> #include <linux/kexec.h>
> +#include <linux/memblock.h>
> #include <linux/of.h>
> #include <linux/ima.h>
> #include "ima.h"
> @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> }
> #endif /* IMA_KEXEC */
>
> +#ifndef CONFIG_OF
> +static phys_addr_t ima_early_kexec_buffer_phys;
> +static size_t ima_early_kexec_buffer_size;
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + if (size == 0)
> + return;
> +
> + ima_early_kexec_buffer_phys = phys_addr;
> + ima_early_kexec_buffer_size = size;
> +}
> +
> +int __init ima_free_kexec_buffer(void)
> +{
> + int rc;
> +
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + rc = memblock_phys_free(ima_early_kexec_buffer_phys,
> + ima_early_kexec_buffer_size);
> + if (rc)
> + return rc;
> +
> + ima_early_kexec_buffer_phys = 0;
> + ima_early_kexec_buffer_size = 0;
> +
> + return 0;
> +}
> +
> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> + return -ENOTSUPP;
> +
> + if (ima_early_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + *addr = __va(ima_early_kexec_buffer_phys);
> + *size = ima_early_kexec_buffer_size;
> +
> + return 0;
> +}
> +
> +#else
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n");
> +}
> +#endif /* CONFIG_OF */
> +
> /*
> * Restore the measurement list from the previous kernel.
> */
> -void ima_load_kexec_buffer(void)
> +void __init ima_load_kexec_buffer(void)
> {
> void *kexec_buffer = NULL;
> size_t kexec_buffer_size = 0;
> --
> 2.34.1
>
On May 9, 2022 10:40:01 AM UTC, Jonathan McDowell <[email protected]> wrote:
>> powerpc and ARM64 both achieve this using device tree with a
>> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
>> device tree
What's wrong with making x86 use the same devicetree node(s)?
--
Sent from a small device: formatting sux and brevity is inevitable.
On Mon, May 09, 2022 at 11:25:22AM +0000, Boris Petkov wrote:
> On May 9, 2022 10:40:01 AM UTC, Jonathan McDowell <[email protected]> wrote:
> >> powerpc and ARM64 both achieve this using device tree with a
> >> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> >> device tree
>
> What's wrong with making x86 use the same devicetree node(s)?
Device tree on x86 doesn't seem to be a thing; none of the distros I
regularly use enable CONFIG_OF for x86, I can only find 2 32-bit x86
platforms that actually select it and none of the plumbing for kexec on
x86 ties in device tree. I agree for platforms that make active use of
device tree that's the appropriate path, but it doesn't seem to be the
case for x86.
J.
On Mon, May 09, 2022 at 05:46:22PM +0000, Jonathan McDowell wrote:
> Device tree on x86 doesn't seem to be a thing;
Not a thing? What does that even mean?
We have arch/x86/kernel/devicetree.c which adds some minimal devicetree
support.
> none of the distros I regularly use enable CONFIG_OF for x86, I can
> only find 2 32-bit x86 platforms that actually select it and none of
> the plumbing for kexec on x86 ties in device tree.
And? That can get changed and enabled and so on.
> I agree for platforms that make active use of device tree that's the
> appropriate path, but it doesn't seem to be the case for x86.
I'm not sure what you're aim here is?
You want to pass that IMA measurement to the kexec kernel with minimal
changes, i.e., change only the kernel?
Why can't distros be also changed to use devicetree for the IMA
measurement, like the other arches do? Why does x86 need to do it
differently?
We also pass info to the kexec kernel by reading it from sysfs
and having kexec tools pass it to the kexec-ed kernel, see
Documentation/ABI/testing/sysfs-firmware-efi-runtime-map
kexec(8) itself can do:
kexec -l kernel-image --append=command-line-options
^^^^^^^^^^^^^^^^^
and add those cmdline options which are dug out from the first kernel.
So is there any particular reason/pressing need to pass the measurement
with setup_data?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, May 09, 2022 at 08:09:55PM +0200, Borislav Petkov wrote:
> On Mon, May 09, 2022 at 05:46:22PM +0000, Jonathan McDowell wrote:
> > Device tree on x86 doesn't seem to be a thing;
>
> Not a thing? What does that even mean?
Let me rephrase more verbosely. Device tree on x86 seems to be a rarely
enabled config option that is only required on a couple of platforms and
lacks the same level of integration with the x86 boot process (compared
to PowerPC and ARM64) such that a) there's a lot more code that would
need to be written to even get to the point that it could be used in the
same manner for passing the IMA buffer as on other platforms and b) I'm
not sure whether or not it might introduce other issues due to this lack
of use/testing.
> We have arch/x86/kernel/devicetree.c which adds some minimal devicetree
> support.
>
> > none of the distros I regularly use enable CONFIG_OF for x86, I can
> > only find 2 32-bit x86 platforms that actually select it and none of
> > the plumbing for kexec on x86 ties in device tree.
>
> And? That can get changed and enabled and so on.
>
> > I agree for platforms that make active use of device tree that's the
> > appropriate path, but it doesn't seem to be the case for x86.
>
> I'm not sure what you're aim here is?
>
> You want to pass that IMA measurement to the kexec kernel with minimal
> changes, i.e., change only the kernel?
I'd like to be able to take an internal kernel buffer and pass it to the
newly kexeced kernel, yes. I'd like that to be possible with the
kexec_file_load() path, which allows for things like kernel signing,
which I believe precludes providing the data from user space.
> Why can't distros be also changed to use devicetree for the IMA
> measurement, like the other arches do? Why does x86 need to do it
> differently?
> We also pass info to the kexec kernel by reading it from sysfs
> and having kexec tools pass it to the kexec-ed kernel, see
> Documentation/ABI/testing/sysfs-firmware-efi-runtime-map
AFAICT kexec passes EFI details using SETUP_EFI, which is what I
modelled the SETUP_IMA support on
(1fec0533693cd74f2d1a46edd29449cfee429df0). In the old syscall variant
the kexec tools do read /sys/firmware/efi/runtime-map and create a
setup_data entry of type SETUP_EFI, in the new file based kexec that is
done inside the kernel in a similar fashion to what I've done with
SETUP_IMA.
> So is there any particular reason/pressing need to pass the measurement
> with setup_data?
I'm not tied to setup_data but given the concerns I raise above with
device tree on x86 and the need to handle this in the kernel it seemed
like a reasonable first approach. You seem to be saying it's not and
either adding the device tree infrastructure or doing a command line
hack would be preferable?
J.
On Mon, May 09, 2022 at 06:41:17PM +0000, Jonathan McDowell wrote:
> I'm not tied to setup_data but given the concerns I raise above with
> device tree on x86 and the need to handle this in the kernel it seemed
> like a reasonable first approach. You seem to be saying it's not and
> either adding the device tree infrastructure or doing a command line
> hack would be preferable?
All I'm doing is asking more questions to make you give more details as
to why you wanna do it this way. I'll take a detailed look tomorrow but
it looks ok from a quick glance.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, May 09, 2022 at 09:40:28PM +0200, Borislav Petkov wrote:
> On Mon, May 09, 2022 at 06:41:17PM +0000, Jonathan McDowell wrote:
> > I'm not tied to setup_data but given the concerns I raise above with
> > device tree on x86 and the need to handle this in the kernel it seemed
> > like a reasonable first approach. You seem to be saying it's not and
> > either adding the device tree infrastructure or doing a command line
> > hack would be preferable?
>
> All I'm doing is asking more questions to make you give more details as
> to why you wanna do it this way. I'll take a detailed look tomorrow but
> it looks ok from a quick glance.
That's reasonable, thanks for taking the time to do so. I realised
another problem with the command line approach is that this is a flow
involving attestation and potentially signing across the kexec boundary,
so if the command line changes every time due to the memory address we
pass the IMA buffer in then we have to recalculate the expected PCR etc
values for every kexec after we've done the user space buffer
allocation, rather than being able to do so once + offline in advance
for a particular kexec across multiple machines.
J.
On Tue, Apr 26, 2022 at 04:52:49PM +0000, Jonathan McDowell wrote:
> Subject: Re: [PATCH v2] Carry forward IMA measurement log on kexec on x86_64
The tip tree preferred format for patch subject prefixes is
'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
'genirq/core:'. Please do not use file names or complete file paths as
prefix. 'git log path/to/file' should give you a reasonable hint in most
cases.
The condensed patch description in the subject line should start with a
uppercase letter and should be written in imperative tone.
I guess in your case
"x86/kexec: Carry ..."
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index b25d3f82c2f3..2f7b138a9388 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -10,6 +10,7 @@
> #define SETUP_EFI 4
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
> +#define SETUP_IMA 7
There's already
#define SETUP_CC_BLOB 7
in the tip tree - please redo your patch against current tip/master.
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..cdc73e081585 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> }
> #endif /* CONFIG_EFI */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void
> +setup_ima_state(const struct kimage *image, struct boot_params *params,
You can push that ugly ifdeffery inside the function like so:
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index cdc73e081585..47ba7083fd44 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,12 +186,12 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
-#ifdef CONFIG_IMA_KEXEC
static void
setup_ima_state(const struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int ima_setup_data_offset)
{
+#ifdef CONFIG_IMA_KEXEC
struct setup_data *sd = (void *)params + ima_setup_data_offset;
struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
unsigned long setup_data_phys;
@@ -209,8 +209,8 @@ setup_ima_state(const struct kimage *image, struct boot_params *params,
setup_data_phys = params_load_addr + ima_setup_data_offset;
sd->next = params->hdr.setup_data;
params->hdr.setup_data = setup_data_phys;
-}
#endif /* CONFIG_IMA_KEXEC */
+}
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
@@ -274,13 +274,11 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
efi_setup_data_offset);
#endif
-#ifdef CONFIG_IMA_KEXEC
/* Setup IMA log buffer state */
setup_ima_state(image, params, params_load_addr,
efi_setup_data_offset +
sizeof(struct setup_data) +
sizeof(struct efi_setup_data));
-#endif
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
> + unsigned long params_load_addr,
> + unsigned int ima_setup_data_offset)
> +{
> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> + struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data);
> + unsigned long setup_data_phys;
The tip-tree preferred ordering of variable declarations at the
beginning of a function is reverse fir tree order::
struct long_struct_name *descriptive_name;
unsigned long foo, bar;
unsigned int tmp;
int ret;
The above is faster to parse than the reverse ordering::
int ret;
unsigned int tmp;
unsigned long foo, bar;
struct long_struct_name *descriptive_name;
And even more so than random ordering::
unsigned long foo, bar;
int ret;
struct long_struct_name *descriptive_name;
unsigned int tmp;
> +
> + if (!image->ima_buffer_size)
> + return;
> +
> + sd->type = SETUP_IMA;
> + sd->len = sizeof(*ima);
> +
> + ima->addr = image->ima_buffer_addr;
> + ima->size = image->ima_buffer_size;
> +
> + /* Add setup data */
> + setup_data_phys = params_load_addr + ima_setup_data_offset;
> + sd->next = params->hdr.setup_data;
> + params->hdr.setup_data = setup_data_phys;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> +
> static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> @@ -247,6 +273,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Setup IMA log buffer state */
> + setup_ima_state(image, params, params_load_addr,
> + efi_setup_data_offset +
> + sizeof(struct setup_data) +
> + sizeof(struct efi_setup_data));
> +#endif
> +
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
> @@ -401,7 +436,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> params_cmdline_sz = ALIGN(params_cmdline_sz, 16);
> kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) +
> sizeof(struct setup_data) +
> - sizeof(struct efi_setup_data);
> + sizeof(struct efi_setup_data) +
> + sizeof(struct setup_data) +
> + sizeof(struct ima_setup_data);
Just because the EFI thing did it unconditionally, regardless of
CONFIG_EFI, you don't have to copy that sloppiness:
unsigned long ima_buf_sz = 0;
...
if (IS_ENABLED(CONFIG_IMA_EXEC))
ima_buf_sz = ...
kbuf.bufsz = ... + ima_buf_sz);
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index c95b9ac5a457..8b0e7725f918 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -11,6 +11,7 @@
> #include <linux/dma-map-ops.h>
> #include <linux/dmi.h>
> #include <linux/efi.h>
> +#include <linux/ima.h>
> #include <linux/init_ohci1394_dma.h>
> #include <linux/initrd.h>
> #include <linux/iscsi_ibft.h>
> @@ -335,6 +336,28 @@ static void __init reserve_initrd(void)
> }
> #endif /* CONFIG_BLK_DEV_INITRD */
>
> +#ifdef CONFIG_IMA_KEXEC
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + struct ima_setup_data *data;
> +
> + data = early_memremap(phys_addr + sizeof(struct setup_data),
> + sizeof(*data));
> + if (!data) {
> + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> + return;
> + }
> + memblock_reserve(data->addr, data->size);
> + ima_set_kexec_buffer(data->addr, data->size);
> + early_memunmap(data, sizeof(*data));
> +}
> +#else
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> + pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
> +}
> +#endif
ditto:
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9324c30755c5..32403d693bf3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -336,9 +336,9 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
-#ifdef CONFIG_IMA_KEXEC
static void __init add_early_ima_buffer(u64 phys_addr)
{
+#ifdef CONFIG_IMA_KEXEC
struct ima_setup_data *data;
data = early_memremap(phys_addr + sizeof(struct setup_data),
@@ -350,13 +350,10 @@ static void __init add_early_ima_buffer(u64 phys_addr)
memblock_reserve(data->addr, data->size);
ima_set_kexec_buffer(data->addr, data->size);
early_memunmap(data, sizeof(*data));
-}
#else
-static void __init add_early_ima_buffer(u64 phys_addr)
-{
pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n");
-}
#endif
+}
static void __init parse_setup_data(void)
{
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 13753136f03f..419c50cfe6b9 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -10,6 +10,7 @@
> #include <linux/seq_file.h>
> #include <linux/vmalloc.h>
> #include <linux/kexec.h>
> +#include <linux/memblock.h>
> #include <linux/of.h>
> #include <linux/ima.h>
> #include "ima.h"
> @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image)
> }
> #endif /* IMA_KEXEC */
>
> +#ifndef CONFIG_OF
> +static phys_addr_t ima_early_kexec_buffer_phys;
> +static size_t ima_early_kexec_buffer_size;
> +
> +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size)
> +{
> + if (size == 0)
> + return;
> +
> + ima_early_kexec_buffer_phys = phys_addr;
> + ima_early_kexec_buffer_size = size;
> +}
> +
> +int __init ima_free_kexec_buffer(void)
WARNING: modpost: vmlinux.o(.text+0xe4e785): Section mismatch in reference from the function ima_free_kexec_buffer() to the function .meminit.text:memblock_phys_free()
The function ima_free_kexec_buffer() references
the function __meminit memblock_phys_free().
This is often because ima_free_kexec_buffer lacks a __meminit
annotation or the annotation of memblock_phys_free is wrong.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
Signed-off-by: Jonathan McDowell <[email protected]>
---
v3:
- Rebase on tip/master
- Pull ima_(free|get)_kexec_buffer into x86 code
- Push ifdefs into functions where possible
- Reverse fir tree variable declarations
- Fix section annotation on ima_free_kexec_buffer (__meminit)
- Only allocate ima_setup_data space when IMA_KEXEC is enabled
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 38 ++++++++++++++++
arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
drivers/of/kexec.c | 1 +
include/linux/ima.h | 3 ++
include/linux/of.h | 2 -
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 118 insertions(+), 6 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f1320d9a3da3..594636f02da4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2027,6 +2027,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdcdf532..ca0796ac4403 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..54bd4ce5f908 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
@@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ efi_setup_data_offset +
+ sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data));
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 249981bf3d8a..ab5e7a351845 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ if (data->size != 0) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_IMA) && !defined(CONFIG_OF_FLATTREE)
+int __meminit ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index b9bd1cff1793..74fdd490f7c0 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..dc6a59d61a15 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,9 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+int ima_free_kexec_buffer(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index 04971e85fbc9..c2f58d2e3a0e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419dc405c831 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.35.3
Hi Jonathan,
On Wed, 2022-05-11 at 09:59 +0000, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> ---
> v3:
> - Rebase on tip/master
This patch doesn't apply to Linus' master branch. Which tip/master
branch? In the future, please use the git format-patch "--base=auto"
option.
> - Pull ima_(free|get)_kexec_buffer into x86 code
> - Push ifdefs into functions where possible
> - Reverse fir tree variable declarations
> - Fix section annotation on ima_free_kexec_buffer (__meminit)
> - Only allocate ima_setup_data space when IMA_KEXEC is enabled
IMA_KEXEC only controls whether the current measurement list should be
carried across kexec, not restoring the previous measurement list.
> v2:
> - Fix operation with EFI systems
> ---
> +++ b/include/linux/ima.h
> @@ -140,6 +140,9 @@ static inline int ima_measure_critical_data(const char *event_label,
>
> #endif /* CONFIG_IMA */
>
> +int ima_free_kexec_buffer(void);
> +int ima_get_kexec_buffer(void **addr, size_t *size);
Wouldn't moving these function definitions here imply they are
implemented on all architectures or are using the version in
drivers/of/kexec.c.
thanks,
Mimi
On Wed, 2022-05-11 at 13:53 -0400, Mimi Zohar wrote:
> Hi Jonathan,
>
> On Wed, 2022-05-11 at 09:59 +0000, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so use the setup_data mechanism to pass the IMA buffer to
> > the new kernel.
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
Thanks, Jonathan. The measurement list is now properly being restored
independently of CONFIG_IMA_KEXEC being configured.
> > +++ b/include/linux/ima.h
> > @@ -140,6 +140,9 @@ static inline int ima_measure_critical_data(const char *event_label,
> >
> > #endif /* CONFIG_IMA */
> >
#ifdef CONFIG_HAVE_IMA_KEXEC
> > +int ima_free_kexec_buffer(void);
> > +int ima_get_kexec_buffer(void **addr, size_t *size);
#endif
>
> Wouldn't moving these function definitions here imply they are
> implemented on all architectures or are using the version in
> drivers/of/kexec.c.
Adding the ifdef around these functions should resolve any issues.
thanks,
Mimi
On Wed, 2022-05-11 at 19:56 +0200, Borislav Petkov wrote:
> On Wed, May 11, 2022 at 01:53:23PM -0400, Mimi Zohar wrote:
> > This patch doesn't apply to Linus' master branch. Which tip/master
> > branch?
>
> This one:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/
thanks!
>
> Considering how the majority of the changes are x86-specific, I was
> thinking I'd carry it through the tip tree after getting your ACK for
> the IMA side of things?
Agreed, all of the changes should be architecture specific. It should
definitely be upstreamed via x86.
thanks,
Mimi
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
Signed-off-by: Jonathan McDowell <[email protected]>
---
v4:
- Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
v3:
- Rebase on tip/master
- Pull ima_(free|get)_kexec_buffer into x86 code
- Push ifdefs into functions where possible
- Reverse fir tree variable declarations
- Fix section annotation on ima_free_kexec_buffer (__meminit)
- Only allocate ima_setup_data space when IMA_KEXEC is enabled
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 38 ++++++++++++++++
arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
drivers/of/kexec.c | 1 +
include/linux/ima.h | 5 +++
include/linux/of.h | 2 -
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 120 insertions(+), 6 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f1320d9a3da3..594636f02da4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2027,6 +2027,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdcdf532..ca0796ac4403 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..54bd4ce5f908 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
@@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
efi_setup_data_offset);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ efi_setup_data_offset +
+ sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data));
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 249981bf3d8a..ab5e7a351845 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ if (data->size != 0) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_IMA) && !defined(CONFIG_OF_FLATTREE)
+int __meminit ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index b9bd1cff1793..74fdd490f7c0 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..ff4bd993e432 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int ima_free_kexec_buffer(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index 04971e85fbc9..c2f58d2e3a0e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419dc405c831 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.35.3
On Wed, May 11, 2022 at 01:53:23PM -0400, Mimi Zohar wrote:
> This patch doesn't apply to Linus' master branch. Which tip/master
> branch?
This one:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/
Considering how the majority of the changes are x86-specific, I was
thinking I'd carry it through the tip tree after getting your ACK for
the IMA side of things?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Jonathan,
On 5/12/2022 9:25 AM, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> ---
> v4:
> - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
> v3:
> - Rebase on tip/master
> - Pull ima_(free|get)_kexec_buffer into x86 code
> - Push ifdefs into functions where possible
> - Reverse fir tree variable declarations
> - Fix section annotation on ima_free_kexec_buffer (__meminit)
> - Only allocate ima_setup_data space when IMA_KEXEC is enabled
> v2:
> - Fix operation with EFI systems
> ---
> arch/x86/Kconfig | 1 +
> arch/x86/include/uapi/asm/bootparam.h | 9 ++++
> arch/x86/kernel/e820.c | 6 +--
> arch/x86/kernel/kexec-bzimage64.c | 38 ++++++++++++++++
> arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
> drivers/of/kexec.c | 1 +
> include/linux/ima.h | 5 +++
> include/linux/of.h | 2 -
> security/integrity/ima/ima_kexec.c | 2 +-
> 9 files changed, 120 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f1320d9a3da3..594636f02da4 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2027,6 +2027,7 @@ config KEXEC_FILE
> bool "kexec file based system call"
> select KEXEC_CORE
> select BUILD_BIN2C
> + select HAVE_IMA_KEXEC if IMA
> depends on X86_64
> depends on CRYPTO=y
> depends on CRYPTO_SHA256=y
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index bea5cdcdf532..ca0796ac4403 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -11,6 +11,7 @@
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
> #define SETUP_CC_BLOB 7
> +#define SETUP_IMA 8
>
> #define SETUP_INDIRECT (1<<31)
>
> @@ -172,6 +173,14 @@ struct jailhouse_setup_data {
> } __attribute__((packed)) v2;
> } __attribute__((packed));
>
> +/*
> + * IMA buffer setup data information from the previous kernel during kexec
> + */
> +struct ima_setup_data {
> + __u64 addr;
> + __u64 size;
> +} __attribute__((packed));
> +
> /* The so-called "zeropage" */
> struct boot_params {
> struct screen_info screen_info; /* 0x000 */
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index f267205f2d5a..9dac24680ff8 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
> e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
>
> /*
> - * SETUP_EFI is supplied by kexec and does not need to be
> - * reserved.
> + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
> + * to be reserved.
> */
> - if (data->type != SETUP_EFI)
> + if (data->type != SETUP_EFI && data->type != SETUP_IMA)
> e820__range_update_kexec(pa_data,
> sizeof(*data) + data->len,
> E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..54bd4ce5f908 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> }
> #endif /* CONFIG_EFI */
>
> +static void
> +setup_ima_state(const struct kimage *image, struct boot_params *params,
> + unsigned long params_load_addr,
> + unsigned int ima_setup_data_offset)
> +{
> +#ifdef CONFIG_IMA_KEXEC
> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> + unsigned long setup_data_phys;
> + struct ima_setup_data *ima;
> +
> + if (!image->ima_buffer_size)
> + return;
> +
> + sd->type = SETUP_IMA;
> + sd->len = sizeof(*ima);
> +
> + ima = (void *)sd + sizeof(struct setup_data);
> + ima->addr = image->ima_buffer_addr;
> + ima->size = image->ima_buffer_size;
> +
> + /* Add setup data */
> + setup_data_phys = params_load_addr + ima_setup_data_offset;
> + sd->next = params->hdr.setup_data;
> + params->hdr.setup_data = setup_data_phys;
> +#endif /* CONFIG_IMA_KEXEC */
> +}
> +
> static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> #endif
> +
> + /* Setup IMA log buffer state */
> + setup_ima_state(image, params, params_load_addr,
> + efi_setup_data_offset +
> + sizeof(struct setup_data) +
> + sizeof(struct efi_setup_data));
Here you could check image->ima_buffer_size and call setup_ima_state()
only if it is non-zero.
> +
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
> @@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> sizeof(struct setup_data) +
> sizeof(struct efi_setup_data);
>
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + kbuf.bufsz += sizeof(struct setup_data) +
> + sizeof(struct ima_setup_data);
> +
> params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> if (!params)
> return ERR_PTR(-ENOMEM);
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 249981bf3d8a..ab5e7a351845 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -11,6 +11,7 @@
> #include <linux/dma-map-ops.h>
> #include <linux/dmi.h>
> #include <linux/efi.h>
> +#include <linux/ima.h>
> #include <linux/init_ohci1394_dma.h>
> #include <linux/initrd.h>
> #include <linux/iscsi_ibft.h>
> @@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
> __visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
> #endif
>
> +#ifdef CONFIG_IMA
> +static phys_addr_t ima_kexec_buffer_phys;
> +static size_t ima_kexec_buffer_size;
> +#endif
> +
> /* Boot loader ID and version as integers, for the benefit of proc_dointvec */
> int bootloader_type, bootloader_version;
>
> @@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
> }
> #endif /* CONFIG_BLK_DEV_INITRD */
>
> +static void __init add_early_ima_buffer(u64 phys_addr)
> +{
> +#ifdef CONFIG_IMA
> + struct ima_setup_data *data;
> +
> + data = early_memremap(phys_addr + sizeof(struct setup_data),
> + sizeof(*data));
> + if (!data) {
> + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> + return;
> + }
Here if memory allocation fails, would kexec system call fail or would
it only not add IMA buffer, but continue with the system call?
> + if (data->size != 0) {
> + memblock_reserve(data->addr, data->size);
> + ima_kexec_buffer_phys = data->addr;
> + ima_kexec_buffer_size = data->size;
> + }
> + early_memunmap(data, sizeof(*data));
> +#else
> + pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
Is this warning message useful? Can we just inline (NOP) this function
if CONFIG_IMA is not set?
> +#endif
> +}
> +
> +#if defined(CONFIG_IMA) && !defined(CONFIG_OF_FLATTREE)
> +int __meminit ima_free_kexec_buffer(void)
> +{
ima_free_kexec_buffer() should be invoked if the previous kernel had
passed the IMA buffer (i.e., CONFIG_HAVE_IMA_KEXEC is set).
CONFIG_HAVE_IMA_KEXEC would be set only if CONFIG_IMA is set. Is the
above check still required?
thanks,
-lakshmi
> + int rc;
> +
> + if (ima_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + rc = memblock_phys_free(ima_kexec_buffer_phys,
> + ima_kexec_buffer_size);
> + if (rc)
> + return rc;
> +
> + ima_kexec_buffer_phys = 0;
> + ima_kexec_buffer_size = 0;
> +
> + return 0;
> +}
> +
> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + if (ima_kexec_buffer_size == 0)
> + return -ENOENT;
> +
> + *addr = __va(ima_kexec_buffer_phys);
> + *size = ima_kexec_buffer_size;
> +
> + return 0;
> +}
> +#endif
> +
> static void __init parse_setup_data(void)
> {
> struct setup_data *data;
> @@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
> case SETUP_EFI:
> parse_efi_setup(pa_data, data_len);
> break;
> + case SETUP_IMA:
> + add_early_ima_buffer(pa_data);
> + break;
> default:
> break;
> }
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index b9bd1cff1793..74fdd490f7c0 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -9,6 +9,7 @@
> * Copyright (C) 2016 IBM Corporation
> */
>
> +#include <linux/ima.h>
> #include <linux/kernel.h>
> #include <linux/kexec.h>
> #include <linux/memblock.h>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 426b1744215e..ff4bd993e432 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
>
> #endif /* CONFIG_IMA */
>
> +#ifdef CONFIG_HAVE_IMA_KEXEC
> +int ima_free_kexec_buffer(void);
> +int ima_get_kexec_buffer(void **addr, size_t *size);
> +#endif
> +
> #ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> extern bool arch_ima_get_secureboot(void);
> extern const char * const *arch_get_ima_policy(void);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 04971e85fbc9..c2f58d2e3a0e 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
> unsigned long initrd_load_addr,
> unsigned long initrd_len,
> const char *cmdline, size_t extra_fdt_size);
> -int ima_get_kexec_buffer(void **addr, size_t *size);
> -int ima_free_kexec_buffer(void);
> #else /* CONFIG_OF */
>
> static inline void of_core_init(void)
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 13753136f03f..419dc405c831 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
> /*
> * Restore the measurement list from the previous kernel.
> */
> -void ima_load_kexec_buffer(void)
> +void __init ima_load_kexec_buffer(void)
> {
> void *kexec_buffer = NULL;
> size_t kexec_buffer_size = 0;
On Fri, May 13, 2022 at 10:19:17AM -0700, Lakshmi Ramasubramanian wrote:
> Hi Jonathan,
>
> On 5/12/2022 9:25 AM, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so use the setup_data mechanism to pass the IMA buffer to
> > the new kernel.
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
> > ---
> > v4:
> > - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
> > v3:
> > - Rebase on tip/master
> > - Pull ima_(free|get)_kexec_buffer into x86 code
> > - Push ifdefs into functions where possible
> > - Reverse fir tree variable declarations
> > - Fix section annotation on ima_free_kexec_buffer (__meminit)
> > - Only allocate ima_setup_data space when IMA_KEXEC is enabled
> > v2:
> > - Fix operation with EFI systems
> > ---
> > arch/x86/Kconfig | 1 +
> > arch/x86/include/uapi/asm/bootparam.h | 9 ++++
> > arch/x86/kernel/e820.c | 6 +--
> > arch/x86/kernel/kexec-bzimage64.c | 38 ++++++++++++++++
> > arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
> > drivers/of/kexec.c | 1 +
> > include/linux/ima.h | 5 +++
> > include/linux/of.h | 2 -
> > security/integrity/ima/ima_kexec.c | 2 +-
> > 9 files changed, 120 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index f1320d9a3da3..594636f02da4 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -2027,6 +2027,7 @@ config KEXEC_FILE
> > bool "kexec file based system call"
> > select KEXEC_CORE
> > select BUILD_BIN2C
> > + select HAVE_IMA_KEXEC if IMA
> > depends on X86_64
> > depends on CRYPTO=y
> > depends on CRYPTO_SHA256=y
> > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> > index bea5cdcdf532..ca0796ac4403 100644
> > --- a/arch/x86/include/uapi/asm/bootparam.h
> > +++ b/arch/x86/include/uapi/asm/bootparam.h
> > @@ -11,6 +11,7 @@
> > #define SETUP_APPLE_PROPERTIES 5
> > #define SETUP_JAILHOUSE 6
> > #define SETUP_CC_BLOB 7
> > +#define SETUP_IMA 8
> > #define SETUP_INDIRECT (1<<31)
> > @@ -172,6 +173,14 @@ struct jailhouse_setup_data {
> > } __attribute__((packed)) v2;
> > } __attribute__((packed));
> > +/*
> > + * IMA buffer setup data information from the previous kernel during kexec
> > + */
> > +struct ima_setup_data {
> > + __u64 addr;
> > + __u64 size;
> > +} __attribute__((packed));
> > +
> > /* The so-called "zeropage" */
> > struct boot_params {
> > struct screen_info screen_info; /* 0x000 */
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index f267205f2d5a..9dac24680ff8 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
> > e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
> > /*
> > - * SETUP_EFI is supplied by kexec and does not need to be
> > - * reserved.
> > + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
> > + * to be reserved.
> > */
> > - if (data->type != SETUP_EFI)
> > + if (data->type != SETUP_EFI && data->type != SETUP_IMA)
> > e820__range_update_kexec(pa_data,
> > sizeof(*data) + data->len,
> > E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > index 170d0fd68b1f..54bd4ce5f908 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> > }
> > #endif /* CONFIG_EFI */
> > +static void
> > +setup_ima_state(const struct kimage *image, struct boot_params *params,
> > + unsigned long params_load_addr,
> > + unsigned int ima_setup_data_offset)
> > +{
> > +#ifdef CONFIG_IMA_KEXEC
> > + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> > + unsigned long setup_data_phys;
> > + struct ima_setup_data *ima;
> > +
> > + if (!image->ima_buffer_size)
> > + return;
> > +
> > + sd->type = SETUP_IMA;
> > + sd->len = sizeof(*ima);
> > +
> > + ima = (void *)sd + sizeof(struct setup_data);
> > + ima->addr = image->ima_buffer_addr;
> > + ima->size = image->ima_buffer_size;
> > +
> > + /* Add setup data */
> > + setup_data_phys = params_load_addr + ima_setup_data_offset;
> > + sd->next = params->hdr.setup_data;
> > + params->hdr.setup_data = setup_data_phys;
> > +#endif /* CONFIG_IMA_KEXEC */
> > +}
> > +
> > static int
> > setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > unsigned long params_load_addr,
> > @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> > efi_setup_data_offset);
> > #endif
> > +
> > + /* Setup IMA log buffer state */
> > + setup_ima_state(image, params, params_load_addr,
> > + efi_setup_data_offset +
> > + sizeof(struct setup_data) +
> > + sizeof(struct efi_setup_data));
> Here you could check image->ima_buffer_size and call setup_ima_state() only
> if it is non-zero.
setup_ima_state() has this check already.
> > +
> > /* Setup EDD info */
> > memcpy(params->eddbuf, boot_params.eddbuf,
> > EDDMAXNR * sizeof(struct edd_info));
> > @@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > sizeof(struct setup_data) +
> > sizeof(struct efi_setup_data);
> > + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> > + kbuf.bufsz += sizeof(struct setup_data) +
> > + sizeof(struct ima_setup_data);
> > +
> > params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> > if (!params)
> > return ERR_PTR(-ENOMEM);
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 249981bf3d8a..ab5e7a351845 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -11,6 +11,7 @@
> > #include <linux/dma-map-ops.h>
> > #include <linux/dmi.h>
> > #include <linux/efi.h>
> > +#include <linux/ima.h>
> > #include <linux/init_ohci1394_dma.h>
> > #include <linux/initrd.h>
> > #include <linux/iscsi_ibft.h>
> > @@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
> > __visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
> > #endif
> > +#ifdef CONFIG_IMA
> > +static phys_addr_t ima_kexec_buffer_phys;
> > +static size_t ima_kexec_buffer_size;
> > +#endif
> > +
> > /* Boot loader ID and version as integers, for the benefit of proc_dointvec */
> > int bootloader_type, bootloader_version;
> > @@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
> > }
> > #endif /* CONFIG_BLK_DEV_INITRD */
> > +static void __init add_early_ima_buffer(u64 phys_addr)
> > +{
> > +#ifdef CONFIG_IMA
> > + struct ima_setup_data *data;
> > +
> > + data = early_memremap(phys_addr + sizeof(struct setup_data),
> > + sizeof(*data));
> > + if (!data) {
> > + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> > + return;
> > + }
> Here if memory allocation fails, would kexec system call fail or would it
> only not add IMA buffer, but continue with the system call?
This is run in the context of the *new* kernel. Boot will continue, but
the IMA buffer will not be successfully passed across. Effectively that
puts us in the same situation as now; things like TPM PCRs will have
been updated, but we won't have the log showing us how we got to the
current state.
> > + if (data->size != 0) {
> > + memblock_reserve(data->addr, data->size);
> > + ima_kexec_buffer_phys = data->addr;
> > + ima_kexec_buffer_size = data->size;
> > + }
> > + early_memunmap(data, sizeof(*data));
> > +#else
> > + pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
> Is this warning message useful? Can we just inline (NOP) this function if
> CONFIG_IMA is not set?
It seems useful to me to know if the previous kernel is trying to pass
us IMA information but we're not configured for IMA, and it's not a lot
of overhead in terms of code in a path that's only actually executed if
we *are* passed the IMA kexec info.
> > +#endif
> > +}
> > +
> > +#if defined(CONFIG_IMA) && !defined(CONFIG_OF_FLATTREE)
> > +int __meminit ima_free_kexec_buffer(void)
> > +{
> ima_free_kexec_buffer() should be invoked if the previous kernel had passed
> the IMA buffer (i.e., CONFIG_HAVE_IMA_KEXEC is set). CONFIG_HAVE_IMA_KEXEC
> would be set only if CONFIG_IMA is set. Is the above check still required?
If we don't have IMA configured there's no point compiling this code in,
as there will be no callers of it. The OF_FLATTREE piece is to handle
the fact that the x86 platforms that use device tree (see previous
discussion in this thread about the fact there only seem to be 2 of
them, and they're both 32 bit) will end up needing to wire up the device
tree kexec passing if they want to use this functionality (and in fact
device tree passing across x86 kexec generally).
> thanks,
> -lakshmi
>
> > + int rc;
> > +
> > + if (ima_kexec_buffer_size == 0)
> > + return -ENOENT;
> > +
> > + rc = memblock_phys_free(ima_kexec_buffer_phys,
> > + ima_kexec_buffer_size);
> > + if (rc)
> > + return rc;
> > +
> > + ima_kexec_buffer_phys = 0;
> > + ima_kexec_buffer_size = 0;
> > +
> > + return 0;
> > +}
> > +
> > +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> > +{
> > + if (ima_kexec_buffer_size == 0)
> > + return -ENOENT;
> > +
> > + *addr = __va(ima_kexec_buffer_phys);
> > + *size = ima_kexec_buffer_size;
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > static void __init parse_setup_data(void)
> > {
> > struct setup_data *data;
> > @@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
> > case SETUP_EFI:
> > parse_efi_setup(pa_data, data_len);
> > break;
> > + case SETUP_IMA:
> > + add_early_ima_buffer(pa_data);
> > + break;
> > default:
> > break;
> > }
> > diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> > index b9bd1cff1793..74fdd490f7c0 100644
> > --- a/drivers/of/kexec.c
> > +++ b/drivers/of/kexec.c
> > @@ -9,6 +9,7 @@
> > * Copyright (C) 2016 IBM Corporation
> > */
> > +#include <linux/ima.h>
> > #include <linux/kernel.h>
> > #include <linux/kexec.h>
> > #include <linux/memblock.h>
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 426b1744215e..ff4bd993e432 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
> > #endif /* CONFIG_IMA */
> > +#ifdef CONFIG_HAVE_IMA_KEXEC
> > +int ima_free_kexec_buffer(void);
> > +int ima_get_kexec_buffer(void **addr, size_t *size);
> > +#endif
> > +
> > #ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
> > extern bool arch_ima_get_secureboot(void);
> > extern const char * const *arch_get_ima_policy(void);
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 04971e85fbc9..c2f58d2e3a0e 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
> > unsigned long initrd_load_addr,
> > unsigned long initrd_len,
> > const char *cmdline, size_t extra_fdt_size);
> > -int ima_get_kexec_buffer(void **addr, size_t *size);
> > -int ima_free_kexec_buffer(void);
> > #else /* CONFIG_OF */
> > static inline void of_core_init(void)
> > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > index 13753136f03f..419dc405c831 100644
> > --- a/security/integrity/ima/ima_kexec.c
> > +++ b/security/integrity/ima/ima_kexec.c
> > @@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
> > /*
> > * Restore the measurement list from the previous kernel.
> > */
> > -void ima_load_kexec_buffer(void)
> > +void __init ima_load_kexec_buffer(void)
> > {
> > void *kexec_buffer = NULL;
> > size_t kexec_buffer_size = 0;
>>> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
>>> index 170d0fd68b1f..54bd4ce5f908 100644
>>> --- a/arch/x86/kernel/kexec-bzimage64.c
>>> +++ b/arch/x86/kernel/kexec-bzimage64.c
>>> @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
>>> }
>>> #endif /* CONFIG_EFI */
>>> +static void
>>> +setup_ima_state(const struct kimage *image, struct boot_params *params,
>>> + unsigned long params_load_addr,
>>> + unsigned int ima_setup_data_offset)
>>> +{
>>> +#ifdef CONFIG_IMA_KEXEC
>>> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
>>> + unsigned long setup_data_phys;
>>> + struct ima_setup_data *ima;
>>> +
>>> + if (!image->ima_buffer_size)
>>> + return;
>>> +
>>> + sd->type = SETUP_IMA;
>>> + sd->len = sizeof(*ima);
>>> +
>>> + ima = (void *)sd + sizeof(struct setup_data);
>>> + ima->addr = image->ima_buffer_addr;
>>> + ima->size = image->ima_buffer_size;
>>> +
>>> + /* Add setup data */
>>> + setup_data_phys = params_load_addr + ima_setup_data_offset;
>>> + sd->next = params->hdr.setup_data;
>>> + params->hdr.setup_data = setup_data_phys;
>>> +#endif /* CONFIG_IMA_KEXEC */
>>> +}
>>> +
>>> static int
>>> setup_boot_parameters(struct kimage *image, struct boot_params *params,
>>> unsigned long params_load_addr,
>>> @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
>>> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
>>> efi_setup_data_offset);
>>> #endif
>>> +
>>> + /* Setup IMA log buffer state */
>>> + setup_ima_state(image, params, params_load_addr,
>>> + efi_setup_data_offset +
>>> + sizeof(struct setup_data) +
>>> + sizeof(struct efi_setup_data));
>> Here you could check image->ima_buffer_size and call setup_ima_state() only
>> if it is non-zero.
>
> setup_ima_state() has this check already.
Yes - I noticed that.
I was just suggesting a minor optimization - avoid making this function
call if IMA buffer is not present.
>
>>> +
>>> /* Setup EDD info */
>>> memcpy(params->eddbuf, boot_params.eddbuf,
>>> EDDMAXNR * sizeof(struct edd_info));
>>> @@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>>> sizeof(struct setup_data) +
>>> sizeof(struct efi_setup_data);
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>>> + kbuf.bufsz += sizeof(struct setup_data) +
>>> + sizeof(struct ima_setup_data);
>>> +
>>> params = kzalloc(kbuf.bufsz, GFP_KERNEL);
>>> if (!params)
>>> return ERR_PTR(-ENOMEM);
>>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>>> index 249981bf3d8a..ab5e7a351845 100644
>>> --- a/arch/x86/kernel/setup.c
>>> +++ b/arch/x86/kernel/setup.c
>>> @@ -11,6 +11,7 @@
>>> #include <linux/dma-map-ops.h>
>>> #include <linux/dmi.h>
>>> #include <linux/efi.h>
>>> +#include <linux/ima.h>
>>> #include <linux/init_ohci1394_dma.h>
>>> #include <linux/initrd.h>
>>> #include <linux/iscsi_ibft.h>
>>> @@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
>>> __visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
>>> #endif
>>> +#ifdef CONFIG_IMA
>>> +static phys_addr_t ima_kexec_buffer_phys;
>>> +static size_t ima_kexec_buffer_size;
>>> +#endif
>>> +
>>> /* Boot loader ID and version as integers, for the benefit of proc_dointvec */
>>> int bootloader_type, bootloader_version;
>>> @@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
>>> }
>>> #endif /* CONFIG_BLK_DEV_INITRD */
>>> +static void __init add_early_ima_buffer(u64 phys_addr)
>>> +{
>>> +#ifdef CONFIG_IMA
>>> + struct ima_setup_data *data;
>>> +
>>> + data = early_memremap(phys_addr + sizeof(struct setup_data),
>>> + sizeof(*data));
>>> + if (!data) {
>>> + pr_warn("setup: failed to memremap ima_setup_data entry\n");
>>> + return;
>>> + }
>> Here if memory allocation fails, would kexec system call fail or would it
>> only not add IMA buffer, but continue with the system call?
>
> This is run in the context of the *new* kernel. Boot will continue, but
> the IMA buffer will not be successfully passed across. Effectively that
> puts us in the same situation as now; things like TPM PCRs will have
> been updated, but we won't have the log showing us how we got to the
> current state.
I think it is better to treat this error as a critical failure.
>
>>> + if (data->size != 0) {
>>> + memblock_reserve(data->addr, data->size);
>>> + ima_kexec_buffer_phys = data->addr;
>>> + ima_kexec_buffer_size = data->size;
>>> + }
>>> + early_memunmap(data, sizeof(*data));
>>> +#else
>>> + pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
>> Is this warning message useful? Can we just inline (NOP) this function if
>> CONFIG_IMA is not set?
>
> It seems useful to me to know if the previous kernel is trying to pass
> us IMA information but we're not configured for IMA, and it's not a lot
> of overhead in terms of code in a path that's only actually executed if
> we *are* passed the IMA kexec info.
okay.
>
>>> +#endif
>>> +}
>>> +
>>> +#if defined(CONFIG_IMA) && !defined(CONFIG_OF_FLATTREE)
>>> +int __meminit ima_free_kexec_buffer(void)
>>> +{
>> ima_free_kexec_buffer() should be invoked if the previous kernel had passed
>> the IMA buffer (i.e., CONFIG_HAVE_IMA_KEXEC is set). CONFIG_HAVE_IMA_KEXEC
>> would be set only if CONFIG_IMA is set. Is the above check still required?
>
> If we don't have IMA configured there's no point compiling this code in,
> as there will be no callers of it. The OF_FLATTREE piece is to handle
> the fact that the x86 platforms that use device tree (see previous
> discussion in this thread about the fact there only seem to be 2 of
> them, and they're both 32 bit) will end up needing to wire up the device
> tree kexec passing if they want to use this functionality (and in fact
> device tree passing across x86 kexec generally).
okay.
-lakshmi
>>
>>> + int rc;
>>> +
>>> + if (ima_kexec_buffer_size == 0)
>>> + return -ENOENT;
>>> +
>>> + rc = memblock_phys_free(ima_kexec_buffer_phys,
>>> + ima_kexec_buffer_size);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + ima_kexec_buffer_phys = 0;
>>> + ima_kexec_buffer_size = 0;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
>>> +{
>>> + if (ima_kexec_buffer_size == 0)
>>> + return -ENOENT;
>>> +
>>> + *addr = __va(ima_kexec_buffer_phys);
>>> + *size = ima_kexec_buffer_size;
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> static void __init parse_setup_data(void)
>>> {
>>> struct setup_data *data;
>>> @@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
>>> case SETUP_EFI:
>>> parse_efi_setup(pa_data, data_len);
>>> break;
>>> + case SETUP_IMA:
>>> + add_early_ima_buffer(pa_data);
>>> + break;
>>> default:
>>> break;
>>> }
>>> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
>>> index b9bd1cff1793..74fdd490f7c0 100644
>>> --- a/drivers/of/kexec.c
>>> +++ b/drivers/of/kexec.c
>>> @@ -9,6 +9,7 @@
>>> * Copyright (C) 2016 IBM Corporation
>>> */
>>> +#include <linux/ima.h>
>>> #include <linux/kernel.h>
>>> #include <linux/kexec.h>
>>> #include <linux/memblock.h>
>>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>>> index 426b1744215e..ff4bd993e432 100644
>>> --- a/include/linux/ima.h
>>> +++ b/include/linux/ima.h
>>> @@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
>>> #endif /* CONFIG_IMA */
>>> +#ifdef CONFIG_HAVE_IMA_KEXEC
>>> +int ima_free_kexec_buffer(void);
>>> +int ima_get_kexec_buffer(void **addr, size_t *size);
>>> +#endif
>>> +
>>> #ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
>>> extern bool arch_ima_get_secureboot(void);
>>> extern const char * const *arch_get_ima_policy(void);
>>> diff --git a/include/linux/of.h b/include/linux/of.h
>>> index 04971e85fbc9..c2f58d2e3a0e 100644
>>> --- a/include/linux/of.h
>>> +++ b/include/linux/of.h
>>> @@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
>>> unsigned long initrd_load_addr,
>>> unsigned long initrd_len,
>>> const char *cmdline, size_t extra_fdt_size);
>>> -int ima_get_kexec_buffer(void **addr, size_t *size);
>>> -int ima_free_kexec_buffer(void);
>>> #else /* CONFIG_OF */
>>> static inline void of_core_init(void)
>>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>>> index 13753136f03f..419dc405c831 100644
>>> --- a/security/integrity/ima/ima_kexec.c
>>> +++ b/security/integrity/ima/ima_kexec.c
>>> @@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>> /*
>>> * Restore the measurement list from the previous kernel.
>>> */
>>> -void ima_load_kexec_buffer(void)
>>> +void __init ima_load_kexec_buffer(void)
>>> {
>>> void *kexec_buffer = NULL;
>>> size_t kexec_buffer_size = 0;
On Tue, May 17, 2022 at 10:19:45AM -0700, Lakshmi Ramasubramanian wrote:
>
> > > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > > > index 170d0fd68b1f..54bd4ce5f908 100644
> > > > --- a/arch/x86/kernel/kexec-bzimage64.c
> > > > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > > > @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> > > > }
> > > > #endif /* CONFIG_EFI */
> > > > +static void
> > > > +setup_ima_state(const struct kimage *image, struct boot_params *params,
> > > > + unsigned long params_load_addr,
> > > > + unsigned int ima_setup_data_offset)
> > > > +{
> > > > +#ifdef CONFIG_IMA_KEXEC
> > > > + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> > > > + unsigned long setup_data_phys;
> > > > + struct ima_setup_data *ima;
> > > > +
> > > > + if (!image->ima_buffer_size)
> > > > + return;
> > > > +
> > > > + sd->type = SETUP_IMA;
> > > > + sd->len = sizeof(*ima);
> > > > +
> > > > + ima = (void *)sd + sizeof(struct setup_data);
> > > > + ima->addr = image->ima_buffer_addr;
> > > > + ima->size = image->ima_buffer_size;
> > > > +
> > > > + /* Add setup data */
> > > > + setup_data_phys = params_load_addr + ima_setup_data_offset;
> > > > + sd->next = params->hdr.setup_data;
> > > > + params->hdr.setup_data = setup_data_phys;
> > > > +#endif /* CONFIG_IMA_KEXEC */
> > > > +}
> > > > +
> > > > static int
> > > > setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > > > unsigned long params_load_addr,
> > > > @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > > > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> > > > efi_setup_data_offset);
> > > > #endif
> > > > +
> > > > + /* Setup IMA log buffer state */
> > > > + setup_ima_state(image, params, params_load_addr,
> > > > + efi_setup_data_offset +
> > > > + sizeof(struct setup_data) +
> > > > + sizeof(struct efi_setup_data));
> > > Here you could check image->ima_buffer_size and call setup_ima_state() only
> > > if it is non-zero.
> >
> > setup_ima_state() has this check already.
>
> Yes - I noticed that.
> I was just suggesting a minor optimization - avoid making this function call
> if IMA buffer is not present.
>
> >
> > > > +
> > > > /* Setup EDD info */
> > > > memcpy(params->eddbuf, boot_params.eddbuf,
> > > > EDDMAXNR * sizeof(struct edd_info));
> > > > @@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > > > sizeof(struct setup_data) +
> > > > sizeof(struct efi_setup_data);
> > > > + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> > > > + kbuf.bufsz += sizeof(struct setup_data) +
> > > > + sizeof(struct ima_setup_data);
> > > > +
> > > > params = kzalloc(kbuf.bufsz, GFP_KERNEL);
> > > > if (!params)
> > > > return ERR_PTR(-ENOMEM);
> > > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > > > index 249981bf3d8a..ab5e7a351845 100644
> > > > --- a/arch/x86/kernel/setup.c
> > > > +++ b/arch/x86/kernel/setup.c
> > > > @@ -11,6 +11,7 @@
> > > > #include <linux/dma-map-ops.h>
> > > > #include <linux/dmi.h>
> > > > #include <linux/efi.h>
> > > > +#include <linux/ima.h>
> > > > #include <linux/init_ohci1394_dma.h>
> > > > #include <linux/initrd.h>
> > > > #include <linux/iscsi_ibft.h>
> > > > @@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
> > > > __visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
> > > > #endif
> > > > +#ifdef CONFIG_IMA
> > > > +static phys_addr_t ima_kexec_buffer_phys;
> > > > +static size_t ima_kexec_buffer_size;
> > > > +#endif
> > > > +
> > > > /* Boot loader ID and version as integers, for the benefit of proc_dointvec */
> > > > int bootloader_type, bootloader_version;
> > > > @@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
> > > > }
> > > > #endif /* CONFIG_BLK_DEV_INITRD */
> > > > +static void __init add_early_ima_buffer(u64 phys_addr)
> > > > +{
> > > > +#ifdef CONFIG_IMA
> > > > + struct ima_setup_data *data;
> > > > +
> > > > + data = early_memremap(phys_addr + sizeof(struct setup_data),
> > > > + sizeof(*data));
> > > > + if (!data) {
> > > > + pr_warn("setup: failed to memremap ima_setup_data entry\n");
> > > > + return;
> > > > + }
> > > Here if memory allocation fails, would kexec system call fail or would it
> > > only not add IMA buffer, but continue with the system call?
> >
> > This is run in the context of the *new* kernel. Boot will continue, but
> > the IMA buffer will not be successfully passed across. Effectively that
> > puts us in the same situation as now; things like TPM PCRs will have
> > been updated, but we won't have the log showing us how we got to the
> > current state.
> I think it is better to treat this error as a critical failure.
That's going to crash the entire system, because it's after we've
started execution of the new kernel. Given that the failure mode will
result in the lack of the logs, but not incorrect TPM PCRs, that seems a
bit extreme.
J.
On Thu, 2022-05-12 at 16:25 +0000, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
Not from using "setup_data" perspective,
Reviewed-by: Mimi Zohar <[email protected]> # IMA function
definitions
thanks,
Mimi
Borislav,
I don't think there are any outstanding review comments for me to deal
with on this, so is it safe to assume it'll get picked up at some point
once the merge window calms down?
On Wed, May 18, 2022 at 10:43:32AM -0400, Mimi Zohar wrote:
> On Thu, 2022-05-12 at 16:25 +0000, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so use the setup_data mechanism to pass the IMA buffer to
> > the new kernel.
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
>
> Not from using "setup_data" perspective,
>
> Reviewed-by: Mimi Zohar <[email protected]> # IMA function
> definitions
>
> thanks,
>
> Mimi
Thanks,
J.
On 5/30/22 01:40, Jonathan McDowell wrote:
> Borislav,
>
> I don't think there are any outstanding review comments for me to deal
> with on this, so is it safe to assume it'll get picked up at some point
> once the merge window calms down?
Nothing here looks too crazy, but it's still been _very_ lightly
reviewed. It doesn't seem like anyone from the kexec world has seen it,
for instance.
Mimi's review was a great start, but it would be really nice to make
sure that the kexec bits look good.
On 06/03/22 at 08:55am, Dave Hansen wrote:
> On 5/30/22 01:40, Jonathan McDowell wrote:
> > Borislav,
> >
> > I don't think there are any outstanding review comments for me to deal
> > with on this, so is it safe to assume it'll get picked up at some point
> > once the merge window calms down?
>
> Nothing here looks too crazy, but it's still been _very_ lightly
> reviewed. It doesn't seem like anyone from the kexec world has seen it,
> for instance.
>
> Mimi's review was a great start, but it would be really nice to make
> sure that the kexec bits look good.
The change looks good from kexec/kdump side. Not sure if Eric has
any concern.
On 05/12/22 at 04:25pm, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
The entire looks good to me, other than a minor concern, please see the
inline comment.
Reviewed-by: Baoquan He <[email protected]>
Hi Coiby,
You can check this patch, see if you can take the same way to solve the
LUKS-encrypted disk issue by passing the key via setup_data.
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> ---
......snip...
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..54bd4ce5f908 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> }
> #endif /* CONFIG_EFI */
>
> +static void
> +setup_ima_state(const struct kimage *image, struct boot_params *params,
> + unsigned long params_load_addr,
> + unsigned int ima_setup_data_offset)
> +{
> +#ifdef CONFIG_IMA_KEXEC
> + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> + unsigned long setup_data_phys;
> + struct ima_setup_data *ima;
> +
> + if (!image->ima_buffer_size)
> + return;
> +
> + sd->type = SETUP_IMA;
> + sd->len = sizeof(*ima);
> +
> + ima = (void *)sd + sizeof(struct setup_data);
> + ima->addr = image->ima_buffer_addr;
> + ima->size = image->ima_buffer_size;
> +
> + /* Add setup data */
> + setup_data_phys = params_load_addr + ima_setup_data_offset;
> + sd->next = params->hdr.setup_data;
> + params->hdr.setup_data = setup_data_phys;
> +#endif /* CONFIG_IMA_KEXEC */
> +}
> +
> static int
> setup_boot_parameters(struct kimage *image, struct boot_params *params,
> unsigned long params_load_addr,
> @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> efi_setup_data_offset);
> #endif
> +
> + /* Setup IMA log buffer state */
> + setup_ima_state(image, params, params_load_addr,
> + efi_setup_data_offset +
> + sizeof(struct setup_data) +
> + sizeof(struct efi_setup_data));
Is it a little better to update efi_setup_data_offset beforehand, or
define a local variable?
efi_setup_data_offset += sizeof(struct setup_data) + sizeof(struct efi_setup_data));
setup_ima_state(image, params, params_load_addr,
efi_setup_data_offset));
No strong opinion. If nobody has concern about it.
> +
> /* Setup EDD info */
> memcpy(params->eddbuf, boot_params.eddbuf,
> EDDMAXNR * sizeof(struct edd_info));
On Mon, Jun 06, 2022 at 12:06:51PM +0800, Baoquan He wrote:
> On 05/12/22 at 04:25pm, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so use the setup_data mechanism to pass the IMA buffer to
> > the new kernel.
>
> The entire looks good to me, other than a minor concern, please see the
> inline comment.
>
> Reviewed-by: Baoquan He <[email protected]>
Thanks. Still waiting to see if Eric has any comments before deciding
whether to spin a v5 or not.
> Hi Coiby,
>
> You can check this patch, see if you can take the same way to solve the
> LUKS-encrypted disk issue by passing the key via setup_data.
>
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
> > ---
> ......snip...
>
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > index 170d0fd68b1f..54bd4ce5f908 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -186,6 +186,33 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> > }
> > #endif /* CONFIG_EFI */
> >
> > +static void
> > +setup_ima_state(const struct kimage *image, struct boot_params *params,
> > + unsigned long params_load_addr,
> > + unsigned int ima_setup_data_offset)
> > +{
> > +#ifdef CONFIG_IMA_KEXEC
> > + struct setup_data *sd = (void *)params + ima_setup_data_offset;
> > + unsigned long setup_data_phys;
> > + struct ima_setup_data *ima;
> > +
> > + if (!image->ima_buffer_size)
> > + return;
> > +
> > + sd->type = SETUP_IMA;
> > + sd->len = sizeof(*ima);
> > +
> > + ima = (void *)sd + sizeof(struct setup_data);
> > + ima->addr = image->ima_buffer_addr;
> > + ima->size = image->ima_buffer_size;
> > +
> > + /* Add setup data */
> > + setup_data_phys = params_load_addr + ima_setup_data_offset;
> > + sd->next = params->hdr.setup_data;
> > + params->hdr.setup_data = setup_data_phys;
> > +#endif /* CONFIG_IMA_KEXEC */
> > +}
> > +
> > static int
> > setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > unsigned long params_load_addr,
> > @@ -247,6 +274,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
> > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
> > efi_setup_data_offset);
> > #endif
> > +
> > + /* Setup IMA log buffer state */
> > + setup_ima_state(image, params, params_load_addr,
> > + efi_setup_data_offset +
> > + sizeof(struct setup_data) +
> > + sizeof(struct efi_setup_data));
>
> Is it a little better to update efi_setup_data_offset beforehand, or
> define a local variable?
>
> efi_setup_data_offset += sizeof(struct setup_data) + sizeof(struct efi_setup_data));
> setup_ima_state(image, params, params_load_addr,
> efi_setup_data_offset));
>
> No strong opinion. If nobody has concern about it.
>
> > +
> > /* Setup EDD info */
> > memcpy(params->eddbuf, boot_params.eddbuf,
> > EDDMAXNR * sizeof(struct edd_info));
>
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
(Mimi, Baoquan, I haven't included your reviewed-bys because this has
changed the compile guards around the ima_(free|get)_kexec_buffer
functions in order to fix the warning the kernel test robot found. I
think this is the right thing to do and avoids us compiling them on
platforms where they won't be used. The alternative would be to drop
the guards in ima.h that Mimi requested for v4.)
Signed-off-by: Jonathan McDowell <[email protected]>
---
v5:
- Guard ima_(free|get)_kexec_buffer functions with
CONFIG_HAVE_IMA_KEXEC (kernel test robot)
- Use setup_data_offset in setup_boot_parameters and update rather than
calculating in call to setup_ima_state.
v4:
- Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
v3:
- Rebase on tip/master
- Pull ima_(free|get)_kexec_buffer into x86 code
- Push ifdefs into functions where possible
- Reverse fir tree variable declarations
- Fix section annotation on ima_free_kexec_buffer (__meminit)
- Only allocate ima_setup_data space when IMA_KEXEC is enabled
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 42 +++++++++++++++++-
arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
drivers/of/kexec.c | 3 ++
include/linux/ima.h | 5 +++
include/linux/of.h | 2 -
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 124 insertions(+), 8 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f6225c9952e8..2cc727125cd6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2034,6 +2034,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdcdf532..ca0796ac4403 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..c63974e94272 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3ebb85327edb..2d0dd8b6588a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -335,6 +341,59 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ if (data->size != 0) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __meminit ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +419,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc552be..42a6c5721a43 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..ff4bd993e432 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int ima_free_kexec_buffer(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index f0a5d6b10c5a..20a4e7cb7afe 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419dc405c831 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.36.1
On Mon, 2022-06-13 at 10:30 +0000, Jonathan McDowell wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> (Mimi, Baoquan, I haven't included your reviewed-bys because this has
> changed the compile guards around the ima_(free|get)_kexec_buffer
> functions in order to fix the warning the kernel test robot found. I
> think this is the right thing to do and avoids us compiling them on
> platforms where they won't be used. The alternative would be to drop
> the guards in ima.h that Mimi requested for v4.)hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> ---
> v5:
> - Guard ima_(free|get)_kexec_buffer functions with
> CONFIG_HAVE_IMA_KEXEC (kernel test robot)
> - Use setup_data_offset in setup_boot_parameters and update rather than
> calculating in call to setup_ima_state.
> v4:
> - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 8d374cc552be..42a6c5721a43 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -9,6 +9,7 @@
> * Copyright (C) 2016 IBM Corporation
> */
>
> +#include <linux/ima.h>
> #include <linux/kernel.h>
> #include <linux/kexec.h>
> #include <linux/memblock.h>
> @@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
> return 0;
> }
>
> +#ifdef CONFIG_HAVE_IMA_KEXEC
> /**
> * ima_get_kexec_buffer - get IMA buffer from the previous kernel
> * @addr: On successful return, set to point to the buffer contents.
> @@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void)
>
> return memblock_phys_free(addr, size);
> }
> +#endif
Inside ima_{get,free}_kexec_buffer(), there's no need now to test
whether CONFIG_HAVE_IMA_KEXEC is enabled.
if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
return -ENOTSUPP;
Otherwise,
Reviewed-by: Mimi Zohar <[email protected]> # IMA function
definitions
>
> /**
> * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
> diff --git a/include/linux/ima.h b/include/linux/ima.h
On 06/13/22 at 05:01pm, Mimi Zohar wrote:
> On Mon, 2022-06-13 at 10:30 +0000, Jonathan McDowell wrote:
> > On kexec file load Integrity Measurement Architecture (IMA) subsystem
> > may verify the IMA signature of the kernel and initramfs, and measure
> > it. The command line parameters passed to the kernel in the kexec call
> > may also be measured by IMA. A remote attestation service can verify
> > a TPM quote based on the TPM event log, the IMA measurement list, and
> > the TPM PCR data. This can be achieved only if the IMA measurement log
> > is carried over from the current kernel to the next kernel across
> > the kexec call.
> >
> > powerpc and ARM64 both achieve this using device tree with a
> > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> > device tree, so use the setup_data mechanism to pass the IMA buffer to
> > the new kernel.
> >
> > (Mimi, Baoquan, I haven't included your reviewed-bys because this has
> > changed the compile guards around the ima_(free|get)_kexec_buffer
> > functions in order to fix the warning the kernel test robot found. I
> > think this is the right thing to do and avoids us compiling them on
> > platforms where they won't be used. The alternative would be to drop
> > the guards in ima.h that Mimi requested for v4.)hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
> >
> > Signed-off-by: Jonathan McDowell <[email protected]>
> > ---
> > v5:
> > - Guard ima_(free|get)_kexec_buffer functions with
> > CONFIG_HAVE_IMA_KEXEC (kernel test robot)
> > - Use setup_data_offset in setup_boot_parameters and update rather than
> > calculating in call to setup_ima_state.
> > v4:
> > - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
>
> > diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> > index 8d374cc552be..42a6c5721a43 100644
> > --- a/drivers/of/kexec.c
> > +++ b/drivers/of/kexec.c
> > @@ -9,6 +9,7 @@
> > * Copyright (C) 2016 IBM Corporation
> > */
> >
> > +#include <linux/ima.h>
> > #include <linux/kernel.h>
> > #include <linux/kexec.h>
> > #include <linux/memblock.h>
> > @@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_HAVE_IMA_KEXEC
> > /**
> > * ima_get_kexec_buffer - get IMA buffer from the previous kernel
> > * @addr: On successful return, set to point to the buffer contents.
> > @@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void)
> >
> > return memblock_phys_free(addr, size);
> > }
> > +#endif
>
> Inside ima_{get,free}_kexec_buffer(), there's no need now to test
> whether CONFIG_HAVE_IMA_KEXEC is enabled.
>
> if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> return -ENOTSUPP;
Indeed. The #ifdef added by Jonathan is redundant. Not sure if the
original IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) checking inside
ima_{get,free}_kexec_buffer() is intended. I ever reviewed below patch,
the IS_ENABLED(CONFIG_XX) inside static function will make the function
compiled, and will be optimized out if the CONFIG_XX is not enabled.
However, it only has effect on static function. Here,
ima_{get,free}_kexec_buffer() is not static, likely we should remove the
inside IS_ENABLED() checking.
commit 4ece09be9913a87ff99ea347fd7e7adad5bdbc8f
Author: Jisheng Zhang <[email protected]>
Date: Wed Mar 23 16:06:39 2022 -0700
x86/setup: use IS_ENABLED(CONFIG_KEXEC_CORE) instead of #ifdef
Replace the conditional compilation using "#ifdef CONFIG_KEXEC_CORE" by a
check for "IS_ENABLED(CONFIG_KEXEC_CORE)", to simplify the code and
increase compile coverage.
Other than this one Mimi pointed out, this patch looks good to me, thx.
Reviewed-by: Baoquan He <[email protected]>
>
>
> >
> > /**
> > * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
>
>
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.
powerpc and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
Signed-off-by: Jonathan McDowell <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]> # IMA function definitions
Reviewed-by: Baoquan He <[email protected]>
---
v6:
- Remove IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) check from
ima_(free|get)_kexec_buffer now they have the ifdef guard.
v5:
- Guard ima_(free|get)_kexec_buffer functions with
CONFIG_HAVE_IMA_KEXEC (kernel test robot)
- Use setup_data_offset in setup_boot_parameters and update rather than
calculating in call to setup_ima_state.
v4:
- Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
v3:
- Rebase on tip/master
- Pull ima_(free|get)_kexec_buffer into x86 code
- Push ifdefs into functions where possible
- Reverse fir tree variable declarations
- Fix section annotation on ima_free_kexec_buffer (__meminit)
- Only allocate ima_setup_data space when IMA_KEXEC is enabled
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 42 +++++++++++++++++-
arch/x86/kernel/setup.c | 62 +++++++++++++++++++++++++++
drivers/of/kexec.c | 9 ++--
include/linux/ima.h | 5 +++
include/linux/of.h | 2 -
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..670e0edc074f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2033,6 +2033,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdcdf532..ca0796ac4403 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..c63974e94272 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bd6c6fd373ae..c9f375045e0d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -140,6 +141,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -330,6 +336,59 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data),
+ sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+ if (data->size != 0) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __meminit ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (ima_kexec_buffer_size == 0)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -355,6 +414,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc552be..d3ec430fa403 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -129,9 +131,6 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
size_t tmp_size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
if (!prop)
return -ENOENT;
@@ -156,9 +155,6 @@ int ima_free_kexec_buffer(void)
size_t size;
struct property *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
if (!prop)
return -ENOENT;
@@ -173,6 +169,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..ff4bd993e432 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int ima_free_kexec_buffer(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index f0a5d6b10c5a..20a4e7cb7afe 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419dc405c831 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.36.1
The following commit has been merged into the x86/kdump branch of tip:
Commit-ID: 69243968bd526641e549ed231c750ce92e3eeb35
Gitweb: https://git.kernel.org/tip/69243968bd526641e549ed231c750ce92e3eeb35
Author: Jonathan McDowell <[email protected]>
AuthorDate: Thu, 16 Jun 2022 15:30:11
Committer: Borislav Petkov <[email protected]>
CommitterDate: Sun, 26 Jun 2022 17:20:11 +02:00
x86/kexec: Carry forward IMA measurement log on kexec
On kexec file load, the Integrity Measurement Architecture (IMA)
subsystem may verify the IMA signature of the kernel and initramfs, and
measure it. The command line parameters passed to the kernel in the
kexec call may also be measured by IMA.
A remote attestation service can verify a TPM quote based on the TPM
event log, the IMA measurement list and the TPM PCR data. This can
be achieved only if the IMA measurement log is carried over from the
current kernel to the next kernel across the kexec call.
PowerPC and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
[ bp: Touchups. ]
Signed-off-by: Jonathan McDowell <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]> # IMA function definitions
Reviewed-by: Baoquan He <[email protected]>
Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
---
arch/x86/Kconfig | 1 +-
arch/x86/include/uapi/asm/bootparam.h | 9 ++++-
arch/x86/kernel/e820.c | 6 +-
arch/x86/kernel/kexec-bzimage64.c | 42 ++++++++++++++++-
arch/x86/kernel/setup.c | 63 ++++++++++++++++++++++++++-
drivers/of/kexec.c | 9 +---
include/linux/ima.h | 5 ++-
include/linux/of.h | 2 +-
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 125 insertions(+), 14 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e..670e0ed 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2033,6 +2033,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdc..ca0796a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205..9dac246 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd..c63974e 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bd6c6fd..482f470 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -140,6 +141,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -330,6 +336,60 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data), sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+
+ if (data->size) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __meminit ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -355,6 +415,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc..d3ec430 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -129,9 +131,6 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
size_t tmp_size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
if (!prop)
return -ENOENT;
@@ -156,9 +155,6 @@ int ima_free_kexec_buffer(void)
size_t size;
struct property *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
if (!prop)
return -ENOENT;
@@ -173,6 +169,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b174..ff4bd99 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int ima_free_kexec_buffer(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index f0a5d6b..20a4e7c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 1375313..419dc40 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
On kexec file load, the Integrity Measurement Architecture (IMA)
subsystem may verify the IMA signature of the kernel and initramfs, and
measure it. The command line parameters passed to the kernel in the
kexec call may also be measured by IMA.
A remote attestation service can verify a TPM quote based on the TPM
event log, the IMA measurement list and the TPM PCR data. This can
be achieved only if the IMA measurement log is carried over from the
current kernel to the next kernel across the kexec call.
PowerPC and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
(Mimi, Baoquan, I haven't included your reviewed-bys because this has
changed the section annotations to __init and Boris reasonably enough
wants to make sure IMA folk are happy before taking this update.)
Signed-off-by: Jonathan McDowell <[email protected]>
Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
---
v7:
- Correct section annotation to __init for ima_free_kexec_buffer + also
on OF variants. (kernel test robot)
v6:
- Remove IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) check from
ima_(free|get)_kexec_buffer now they have the ifdef guard.
v5:
- Guard ima_(free|get)_kexec_buffer functions with
CONFIG_HAVE_IMA_KEXEC (kernel test robot)
- Use setup_data_offset in setup_boot_parameters and update rather than
calculating in call to setup_ima_state.
v4:
- Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC
v3:
- Rebase on tip/master
- Pull ima_(free|get)_kexec_buffer into x86 code
- Push ifdefs into functions where possible
- Reverse fir tree variable declarations
- Fix section annotation on ima_free_kexec_buffer (__meminit)
- Only allocate ima_setup_data space when IMA_KEXEC is enabled
v2:
- Fix operation with EFI systems
---
arch/x86/Kconfig | 1 +
arch/x86/include/uapi/asm/bootparam.h | 9 ++++
arch/x86/kernel/e820.c | 6 +--
arch/x86/kernel/kexec-bzimage64.c | 42 +++++++++++++++++-
arch/x86/kernel/setup.c | 63 +++++++++++++++++++++++++++
drivers/of/kexec.c | 13 +++---
include/linux/ima.h | 5 +++
include/linux/of.h | 2 -
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 127 insertions(+), 16 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e51df6..670e0edc074f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2033,6 +2033,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdcdf532..ca0796ac4403 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..9dac24680ff8 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..c63974e94272 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bd6c6fd373ae..53f863f28b4c 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -140,6 +141,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -330,6 +336,60 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data), sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+
+ if (data->size) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __init ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -355,6 +415,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc552be..f2e58ddfaed2 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -122,16 +124,13 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
*
* Return: 0 on success, negative errno on error.
*/
-int ima_get_kexec_buffer(void **addr, size_t *size)
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
{
int ret, len;
unsigned long tmp_addr;
size_t tmp_size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
if (!prop)
return -ENOENT;
@@ -149,16 +148,13 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
/**
* ima_free_kexec_buffer - free memory used by the IMA buffer
*/
-int ima_free_kexec_buffer(void)
+int __init ima_free_kexec_buffer(void)
{
int ret;
unsigned long addr;
size_t size;
struct property *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
if (!prop)
return -ENOENT;
@@ -173,6 +169,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b1744215e..81708ca0ebc7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int __init ima_free_kexec_buffer(void);
+int __init ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index f0a5d6b10c5a..20a4e7cb7afe 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 13753136f03f..419dc405c831 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
--
2.36.1
On Thu, 2022-06-30 at 08:36 +0000, Jonathan McDowell wrote:
> On kexec file load, the Integrity Measurement Architecture (IMA)
> subsystem may verify the IMA signature of the kernel and initramfs, and
> measure it. The command line parameters passed to the kernel in the
> kexec call may also be measured by IMA.
>
> A remote attestation service can verify a TPM quote based on the TPM
> event log, the IMA measurement list and the TPM PCR data. This can
> be achieved only if the IMA measurement log is carried over from the
> current kernel to the next kernel across the kexec call.
>
> PowerPC and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> (Mimi, Baoquan, I haven't included your reviewed-bys because this has
> changed the section annotations to __init and Boris reasonably enough
> wants to make sure IMA folk are happy before taking this update.)
FYI, comments like this should be added before the patch changelog
(after the '---' separator).
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
Reviewed-by: Mimi Zohar <[email protected]> # IMA function
definitions
The following commit has been merged into the x86/kdump branch of tip:
Commit-ID: b69a2afd5afce9bf6d56e349d6ab592c916e20f2
Gitweb: https://git.kernel.org/tip/b69a2afd5afce9bf6d56e349d6ab592c916e20f2
Author: Jonathan McDowell <[email protected]>
AuthorDate: Thu, 30 Jun 2022 08:36:12
Committer: Borislav Petkov <[email protected]>
CommitterDate: Fri, 01 Jul 2022 15:22:16 +02:00
x86/kexec: Carry forward IMA measurement log on kexec
On kexec file load, the Integrity Measurement Architecture (IMA)
subsystem may verify the IMA signature of the kernel and initramfs, and
measure it. The command line parameters passed to the kernel in the
kexec call may also be measured by IMA.
A remote attestation service can verify a TPM quote based on the TPM
event log, the IMA measurement list and the TPM PCR data. This can
be achieved only if the IMA measurement log is carried over from the
current kernel to the next kernel across the kexec call.
PowerPC and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
Signed-off-by: Jonathan McDowell <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]> # IMA function definitions
Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
---
arch/x86/Kconfig | 1 +-
arch/x86/include/uapi/asm/bootparam.h | 9 ++++-
arch/x86/kernel/e820.c | 6 +-
arch/x86/kernel/kexec-bzimage64.c | 42 ++++++++++++++++-
arch/x86/kernel/setup.c | 63 ++++++++++++++++++++++++++-
drivers/of/kexec.c | 13 +----
include/linux/ima.h | 5 ++-
include/linux/of.h | 2 +-
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 127 insertions(+), 16 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index be0b95e..670e0ed 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2033,6 +2033,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdc..ca0796a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205..9dac246 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd..c63974e 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index bd6c6fd..53f863f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -140,6 +141,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -330,6 +336,60 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data), sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+
+ if (data->size) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __init ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -355,6 +415,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc..f2e58dd 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -122,16 +124,13 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
*
* Return: 0 on success, negative errno on error.
*/
-int ima_get_kexec_buffer(void **addr, size_t *size)
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
{
int ret, len;
unsigned long tmp_addr;
size_t tmp_size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
if (!prop)
return -ENOENT;
@@ -149,16 +148,13 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
/**
* ima_free_kexec_buffer - free memory used by the IMA buffer
*/
-int ima_free_kexec_buffer(void)
+int __init ima_free_kexec_buffer(void)
{
int ret;
unsigned long addr;
size_t size;
struct property *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
if (!prop)
return -ENOENT;
@@ -173,6 +169,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b174..81708ca 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int __init ima_free_kexec_buffer(void);
+int __init ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index f0a5d6b..20a4e7c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 1375313..419dc40 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
On 06/30/22 at 08:36am, Jonathan McDowell wrote:
> On kexec file load, the Integrity Measurement Architecture (IMA)
> subsystem may verify the IMA signature of the kernel and initramfs, and
> measure it. The command line parameters passed to the kernel in the
> kexec call may also be measured by IMA.
>
> A remote attestation service can verify a TPM quote based on the TPM
> event log, the IMA measurement list and the TPM PCR data. This can
> be achieved only if the IMA measurement log is carried over from the
> current kernel to the next kernel across the kexec call.
>
> PowerPC and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> (Mimi, Baoquan, I haven't included your reviewed-bys because this has
> changed the section annotations to __init and Boris reasonably enough
> wants to make sure IMA folk are happy before taking this update.)
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
LGTM,
Reviewed-by: Baoquan He <[email protected]>
The following commit has been merged into the x86/boot branch of tip:
Commit-ID: 2faaa8f3ef16d794ecb28f9a7d9dca25cff98bb3
Gitweb: https://git.kernel.org/tip/2faaa8f3ef16d794ecb28f9a7d9dca25cff98bb3
Author: Jonathan McDowell <[email protected]>
AuthorDate: Thu, 30 Jun 2022 08:36:12
Committer: Dave Hansen <[email protected]>
CommitterDate: Wed, 06 Jul 2022 15:45:55 -07:00
x86/kexec: Carry forward IMA measurement log on kexec
On kexec file load, the Integrity Measurement Architecture (IMA)
subsystem may verify the IMA signature of the kernel and initramfs, and
measure it. The command line parameters passed to the kernel in the
kexec call may also be measured by IMA.
A remote attestation service can verify a TPM quote based on the TPM
event log, the IMA measurement list and the TPM PCR data. This can
be achieved only if the IMA measurement log is carried over from the
current kernel to the next kernel across the kexec call.
PowerPC and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.
(Mimi, Baoquan, I haven't included your reviewed-bys because this has
changed the section annotations to __init and Boris reasonably enough
wants to make sure IMA folk are happy before taking this update.)
Signed-off-by: Jonathan McDowell <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]> # IMA function
Reviewed-by: Baoquan He <[email protected]>
Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/Kconfig | 1 +-
arch/x86/include/uapi/asm/bootparam.h | 9 ++++-
arch/x86/kernel/e820.c | 6 +-
arch/x86/kernel/kexec-bzimage64.c | 42 ++++++++++++++++-
arch/x86/kernel/setup.c | 63 ++++++++++++++++++++++++++-
drivers/of/kexec.c | 13 +----
include/linux/ima.h | 5 ++-
include/linux/of.h | 2 +-
security/integrity/ima/ima_kexec.c | 2 +-
9 files changed, 127 insertions(+), 16 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 762a0b6..5465def 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2027,6 +2027,7 @@ config KEXEC_FILE
bool "kexec file based system call"
select KEXEC_CORE
select BUILD_BIN2C
+ select HAVE_IMA_KEXEC if IMA
depends on X86_64
depends on CRYPTO=y
depends on CRYPTO_SHA256=y
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index bea5cdc..ca0796a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -11,6 +11,7 @@
#define SETUP_APPLE_PROPERTIES 5
#define SETUP_JAILHOUSE 6
#define SETUP_CC_BLOB 7
+#define SETUP_IMA 8
#define SETUP_INDIRECT (1<<31)
@@ -172,6 +173,14 @@ struct jailhouse_setup_data {
} __attribute__((packed)) v2;
} __attribute__((packed));
+/*
+ * IMA buffer setup data information from the previous kernel during kexec
+ */
+struct ima_setup_data {
+ __u64 addr;
+ __u64 size;
+} __attribute__((packed));
+
/* The so-called "zeropage" */
struct boot_params {
struct screen_info screen_info; /* 0x000 */
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205..9dac246 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void)
e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
/*
- * SETUP_EFI is supplied by kexec and does not need to be
- * reserved.
+ * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need
+ * to be reserved.
*/
- if (data->type != SETUP_EFI)
+ if (data->type != SETUP_EFI && data->type != SETUP_IMA)
e820__range_update_kexec(pa_data,
sizeof(*data) + data->len,
E820_TYPE_RAM, E820_TYPE_RESERVED_KERN);
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd..c63974e 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -186,11 +186,38 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
}
#endif /* CONFIG_EFI */
+static void
+setup_ima_state(const struct kimage *image, struct boot_params *params,
+ unsigned long params_load_addr,
+ unsigned int ima_setup_data_offset)
+{
+#ifdef CONFIG_IMA_KEXEC
+ struct setup_data *sd = (void *)params + ima_setup_data_offset;
+ unsigned long setup_data_phys;
+ struct ima_setup_data *ima;
+
+ if (!image->ima_buffer_size)
+ return;
+
+ sd->type = SETUP_IMA;
+ sd->len = sizeof(*ima);
+
+ ima = (void *)sd + sizeof(struct setup_data);
+ ima->addr = image->ima_buffer_addr;
+ ima->size = image->ima_buffer_size;
+
+ /* Add setup data */
+ setup_data_phys = params_load_addr + ima_setup_data_offset;
+ sd->next = params->hdr.setup_data;
+ params->hdr.setup_data = setup_data_phys;
+#endif /* CONFIG_IMA_KEXEC */
+}
+
static int
setup_boot_parameters(struct kimage *image, struct boot_params *params,
unsigned long params_load_addr,
unsigned int efi_map_offset, unsigned int efi_map_sz,
- unsigned int efi_setup_data_offset)
+ unsigned int setup_data_offset)
{
unsigned int nr_e820_entries;
unsigned long long mem_k, start, end;
@@ -245,8 +272,15 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
#ifdef CONFIG_EFI
/* Setup EFI state */
setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz,
- efi_setup_data_offset);
+ setup_data_offset);
+ setup_data_offset += sizeof(struct setup_data) +
+ sizeof(struct efi_setup_data);
#endif
+
+ /* Setup IMA log buffer state */
+ setup_ima_state(image, params, params_load_addr,
+ setup_data_offset);
+
/* Setup EDD info */
memcpy(params->eddbuf, boot_params.eddbuf,
EDDMAXNR * sizeof(struct edd_info));
@@ -403,6 +437,10 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
sizeof(struct setup_data) +
sizeof(struct efi_setup_data);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ kbuf.bufsz += sizeof(struct setup_data) +
+ sizeof(struct ima_setup_data);
+
params = kzalloc(kbuf.bufsz, GFP_KERNEL);
if (!params)
return ERR_PTR(-ENOMEM);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3ebb853..e21ad55 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
#include <linux/dma-map-ops.h>
#include <linux/dmi.h>
#include <linux/efi.h>
+#include <linux/ima.h>
#include <linux/init_ohci1394_dma.h>
#include <linux/initrd.h>
#include <linux/iscsi_ibft.h>
@@ -145,6 +146,11 @@ __visible unsigned long mmu_cr4_features __ro_after_init;
__visible unsigned long mmu_cr4_features __ro_after_init = X86_CR4_PAE;
#endif
+#ifdef CONFIG_IMA
+static phys_addr_t ima_kexec_buffer_phys;
+static size_t ima_kexec_buffer_size;
+#endif
+
/* Boot loader ID and version as integers, for the benefit of proc_dointvec */
int bootloader_type, bootloader_version;
@@ -335,6 +341,60 @@ static void __init reserve_initrd(void)
}
#endif /* CONFIG_BLK_DEV_INITRD */
+static void __init add_early_ima_buffer(u64 phys_addr)
+{
+#ifdef CONFIG_IMA
+ struct ima_setup_data *data;
+
+ data = early_memremap(phys_addr + sizeof(struct setup_data), sizeof(*data));
+ if (!data) {
+ pr_warn("setup: failed to memremap ima_setup_data entry\n");
+ return;
+ }
+
+ if (data->size) {
+ memblock_reserve(data->addr, data->size);
+ ima_kexec_buffer_phys = data->addr;
+ ima_kexec_buffer_size = data->size;
+ }
+
+ early_memunmap(data, sizeof(*data));
+#else
+ pr_warn("Passed IMA kexec data, but CONFIG_IMA not set. Ignoring.\n");
+#endif
+}
+
+#if defined(CONFIG_HAVE_IMA_KEXEC) && !defined(CONFIG_OF_FLATTREE)
+int __init ima_free_kexec_buffer(void)
+{
+ int rc;
+
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ rc = memblock_phys_free(ima_kexec_buffer_phys,
+ ima_kexec_buffer_size);
+ if (rc)
+ return rc;
+
+ ima_kexec_buffer_phys = 0;
+ ima_kexec_buffer_size = 0;
+
+ return 0;
+}
+
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
+{
+ if (!ima_kexec_buffer_size)
+ return -ENOENT;
+
+ *addr = __va(ima_kexec_buffer_phys);
+ *size = ima_kexec_buffer_size;
+
+ return 0;
+}
+#endif
+
static void __init parse_setup_data(void)
{
struct setup_data *data;
@@ -360,6 +420,9 @@ static void __init parse_setup_data(void)
case SETUP_EFI:
parse_efi_setup(pa_data, data_len);
break;
+ case SETUP_IMA:
+ add_early_ima_buffer(pa_data);
+ break;
default:
break;
}
diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc..f2e58dd 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -9,6 +9,7 @@
* Copyright (C) 2016 IBM Corporation
*/
+#include <linux/ima.h>
#include <linux/kernel.h>
#include <linux/kexec.h>
#include <linux/memblock.h>
@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
return 0;
}
+#ifdef CONFIG_HAVE_IMA_KEXEC
/**
* ima_get_kexec_buffer - get IMA buffer from the previous kernel
* @addr: On successful return, set to point to the buffer contents.
@@ -122,16 +124,13 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
*
* Return: 0 on success, negative errno on error.
*/
-int ima_get_kexec_buffer(void **addr, size_t *size)
+int __init ima_get_kexec_buffer(void **addr, size_t *size)
{
int ret, len;
unsigned long tmp_addr;
size_t tmp_size;
const void *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
if (!prop)
return -ENOENT;
@@ -149,16 +148,13 @@ int ima_get_kexec_buffer(void **addr, size_t *size)
/**
* ima_free_kexec_buffer - free memory used by the IMA buffer
*/
-int ima_free_kexec_buffer(void)
+int __init ima_free_kexec_buffer(void)
{
int ret;
unsigned long addr;
size_t size;
struct property *prop;
- if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
- return -ENOTSUPP;
-
prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
if (!prop)
return -ENOENT;
@@ -173,6 +169,7 @@ int ima_free_kexec_buffer(void)
return memblock_phys_free(addr, size);
}
+#endif
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 426b174..81708ca 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -140,6 +140,11 @@ static inline int ima_measure_critical_data(const char *event_label,
#endif /* CONFIG_IMA */
+#ifdef CONFIG_HAVE_IMA_KEXEC
+int __init ima_free_kexec_buffer(void);
+int __init ima_get_kexec_buffer(void **addr, size_t *size);
+#endif
+
#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
diff --git a/include/linux/of.h b/include/linux/of.h
index 04971e8..c2f58d2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -441,8 +441,6 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline, size_t extra_fdt_size);
-int ima_get_kexec_buffer(void **addr, size_t *size);
-int ima_free_kexec_buffer(void);
#else /* CONFIG_OF */
static inline void of_core_init(void)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 1375313..419dc40 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -137,7 +137,7 @@ void ima_add_kexec_buffer(struct kimage *image)
/*
* Restore the measurement list from the previous kernel.
*/
-void ima_load_kexec_buffer(void)
+void __init ima_load_kexec_buffer(void)
{
void *kexec_buffer = NULL;
size_t kexec_buffer_size = 0;
On 7/7/22 10:37, Jonathan McDowell wrote:
> On Thu, Jul 07, 2022 at 04:52:32PM -0000, tip-bot2 for Jonathan McDowell wrote:
>> The following commit has been merged into the x86/boot branch of tip:
> Just to clarify there's not some confusion going on; this is already in
> -next via tip/master via tip/x86/kdump.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/kdump&id=b69a2afd5afce9bf6d56e349d6ab592c916e20f2
Ahh, thanks for the heads up. I'll zap it from x86/boot.
On Thu, Jul 07, 2022 at 04:52:32PM -0000, tip-bot2 for Jonathan McDowell wrote:
> The following commit has been merged into the x86/boot branch of tip:
Just to clarify there's not some confusion going on; this is already in
-next via tip/master via tip/x86/kdump.
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/kdump&id=b69a2afd5afce9bf6d56e349d6ab592c916e20f2
> Commit-ID: 2faaa8f3ef16d794ecb28f9a7d9dca25cff98bb3
> Gitweb: https://git.kernel.org/tip/2faaa8f3ef16d794ecb28f9a7d9dca25cff98bb3
> Author: Jonathan McDowell <[email protected]>
> AuthorDate: Thu, 30 Jun 2022 08:36:12
> Committer: Dave Hansen <[email protected]>
> CommitterDate: Wed, 06 Jul 2022 15:45:55 -07:00
>
> x86/kexec: Carry forward IMA measurement log on kexec
>
> On kexec file load, the Integrity Measurement Architecture (IMA)
> subsystem may verify the IMA signature of the kernel and initramfs, and
> measure it. The command line parameters passed to the kernel in the
> kexec call may also be measured by IMA.
>
> A remote attestation service can verify a TPM quote based on the TPM
> event log, the IMA measurement list and the TPM PCR data. This can
> be achieved only if the IMA measurement log is carried over from the
> current kernel to the next kernel across the kexec call.
>
> PowerPC and ARM64 both achieve this using device tree with a
> "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
> device tree, so use the setup_data mechanism to pass the IMA buffer to
> the new kernel.
>
> (Mimi, Baoquan, I haven't included your reviewed-bys because this has
> changed the section annotations to __init and Boris reasonably enough
> wants to make sure IMA folk are happy before taking this update.)
>
> Signed-off-by: Jonathan McDowell <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> Reviewed-by: Mimi Zohar <[email protected]> # IMA function
> Reviewed-by: Baoquan He <[email protected]>
> Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG
> Link: https://lkml.kernel.org/r/[email protected]