[RFC PATCH v1 0/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[RFC PATCH v1 0/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Prakhar Srivastava]

Add support for arm64 to carry ima measurement log
to the next kexec'ed session triggered via kexec_file_load.
- Top of Linux 5.3-rc6

Currently during kexec the kernel file signatures are/can be validated
prior to actual load, the information(PE/ima signature) is not carried
to the next session. This lead to loss of information.

Carrying forward the ima measurement log to the next kexec'ed session 
allows a verifying party to get the entire runtime event log since the
last full reboot, since that is when PCRs were last reset.

Changelog:

v1:
  - add new fdt porperties to mark start and end for ima measurement
    log.
  - use fdt_* functions to add/remove fdt properties and memory
    allocations.
  - remove additional check for endian-ness as they are checked
    in fdt_* functions.

v0:
  - Add support to carry ima measurement log in arm64, 
   uses same code as powerpc.

Prakhar Srivastava (1):
  Add support for arm64 to carry ima measurement log in kexec_file_load

 arch/arm64/Kconfig                     |   7 +
 arch/arm64/include/asm/ima.h           |  29 ++++
 arch/arm64/include/asm/kexec.h         |   5 +
 arch/arm64/kernel/Makefile             |   3 +-
 arch/arm64/kernel/ima_kexec.c          | 213 +++++++++++++++++++++++++
 arch/arm64/kernel/machine_kexec_file.c |   6 +
 6 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c

-- 
2.17.1

[RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Prakhar Srivastava]

During kexec_file_load, carrying forward the ima measurement log allows
a verifying party to get the entire runtime event log since the last
full reboot since that is when PCRs were last reset.

Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
---
 arch/arm64/Kconfig                     |   7 +
 arch/arm64/include/asm/ima.h           |  29 ++++
 arch/arm64/include/asm/kexec.h         |   5 +
 arch/arm64/kernel/Makefile             |   3 +-
 arch/arm64/kernel/ima_kexec.c          | 213 +++++++++++++++++++++++++
 arch/arm64/kernel/machine_kexec_file.c |   6 +
 6 files changed, 262 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3adcec05b1f6..f39b12dbf9e8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
 	  verification for the corresponding kernel image type being
 	  loaded in order for this to work.
 
+config HAVE_IMA_KEXEC
+	bool "Carry over IMA measurement log during kexec_file_load() syscall"
+	depends on KEXEC_FILE
+	help
+	  Select this option to carry over IMA measurement log during
+	  kexec_file_load.
+
 config KEXEC_IMAGE_VERIFY_SIG
 	bool "Enable Image signature verification support"
 	default y
diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
new file mode 100644
index 000000000000..e23cee84729f
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_IMA_H
+#define _ASM_ARM64_IMA_H
+
+struct kimage;
+
+int ima_get_kexec_buffer(void **addr, size_t *size);
+int ima_free_kexec_buffer(void);
+
+#ifdef CONFIG_IMA
+void remove_ima_buffer(void *fdt, int chosen_node);
+#else
+static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
+#endif
+
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      size_t size);
+
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
+#else
+static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
+				   int chosen_node)
+{
+	remove_ima_buffer(fdt, chosen_node);
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARM64_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index 12a561a54128..e8d2412066e7 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -96,6 +96,11 @@ static inline void crash_post_resume(void) {}
 struct kimage_arch {
 	void *dtb;
 	unsigned long dtb_mem;
+
+#ifdef CONFIG_IMA_KEXEC
+	phys_addr_t ima_buffer_addr;
+	size_t ima_buffer_size;
+#endif
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 478491f07b4f..580238f2e9a7 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -55,7 +55,8 @@ obj-$(CONFIG_RANDOMIZE_BASE)		+= kaslr.o
 obj-$(CONFIG_HIBERNATION)		+= hibernate.o hibernate-asm.o
 obj-$(CONFIG_KEXEC_CORE)		+= machine_kexec.o relocate_kernel.o	\
 					   cpu-reset.o
-obj-$(CONFIG_KEXEC_FILE)		+= machine_kexec_file.o kexec_image.o
+obj-$(CONFIG_KEXEC_FILE)		+= machine_kexec_file.o kexec_image.o	\
+					   ima_kexec.o
 obj-$(CONFIG_ARM64_RELOC_TEST)		+= arm64-reloc-test.o
 arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
 obj-$(CONFIG_CRASH_DUMP)		+= crash_dump.o
diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
new file mode 100644
index 000000000000..b14326d541f3
--- /dev/null
+++ b/arch/arm64/kernel/ima_kexec.c
@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Microsoft Corporation.
+ *
+ * Authors:
+ * Prakhar Srivastava <prsriva@linux.microsoft.com>
+ */
+
+#include <linux/slab.h>
+#include <linux/kexec.h>
+#include <linux/of.h>
+#include <linux/memblock.h>
+#include <linux/libfdt.h>
+
+
+/**
+ * delete_fdt_mem_rsv - delete memory reservation with given address and size
+ * @fdt - pointer to the fdt.
+ * @start - start address of the memory.
+ * @size - number of cells to be deletd.
+ * 
+ * Return: 0 on success, or negative errno on error.
+ */
+int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+{
+	int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
+
+	for (i = 0; i < num_rsvs; i++) {
+		uint64_t rsv_start, rsv_size;
+
+		ret = fdt_get_mem_rsv(fdt, i, &rsv_start, &rsv_size);
+		if (ret) {
+			pr_err("Malformed device tree\n");
+			return -EINVAL;
+		}
+
+		if (rsv_start == start && rsv_size == size) {
+			ret = fdt_del_mem_rsv(fdt, i);
+			if (ret) {
+				pr_err("Error deleting device tree reservation\n");
+				return -EINVAL;
+			}
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+/**
+ * remove_ima_buffer - remove the IMA buffer property and reservation
+ * @fdt - pointer the fdt.
+ * @chosen_node - node under which property can be found.
+ * 
+ * The IMA measurement buffer is either read by now and freeed or a kexec call
+ * needs to replace the ima measurement buffer, clear the property and memory
+ * reservation.
+ */
+void remove_ima_buffer(void *fdt, int chosen_node)
+{
+	int ret, len;
+	const void *prop;
+	uint64_t tmp_start, tmp_end;
+
+	prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
+	if (prop) {
+		tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+		prop = fdt_getprop(fdt, chosen_node,
+				   "linux,ima-kexec-buffer-end", &len);
+		if (!prop)
+			return;
+
+		tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+		ret = delete_fdt_mem_rsv(fdt, tmp_start, tmp_end - tmp_start);
+
+		if (ret == 0)
+			pr_debug("Removed old IMA buffer reservation.\n");
+		else if (ret != -ENOENT)
+			return;
+
+		fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
+		fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer-end");
+	}
+}
+
+/**
+ * ima_get_kexec_buffer - get IMA buffer from the previous kernel
+ * @addr:	On successful return, set to point to the buffer contents.
+ * @size:	On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_get_kexec_buffer(void **addr, size_t *size)
+{
+	int len;
+	const void *prop;
+	uint64_t tmp_start, tmp_end;
+
+	prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
+	if (!prop)
+		return -ENOENT;
+
+	tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+	prop = of_get_property(of_chosen, "linux,ima-kexec-buffer-end", &len);
+	if (!prop)
+		return -ENOENT;
+
+	tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
+
+	*addr = __va(tmp_start);
+	*size = tmp_end - tmp_start;
+
+	return 0;
+}
+
+/**
+ * ima_free_kexec_buffer - free memory used by the IMA buffer
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_free_kexec_buffer(void)
+{
+	int ret;
+	void *propStart, *propEnd;
+	uint64_t tmp_start, tmp_end;
+
+	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
+				     NULL);
+	if (propStart) {
+		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
+		ret = of_remove_property(of_chosen, propStart);
+		if (!ret) {
+			return ret;
+		}
+
+		propEnd = of_find_property(of_chosen,
+					   "linux,ima-kexec-buffer-end", NULL);
+		if (!propEnd) {
+			return -EINVAL;
+		}
+
+		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
+
+		ret = of_remove_property(of_chosen, propEnd);
+		if (!ret) {
+			return ret;
+		}
+
+		return memblock_free(tmp_start, tmp_end - tmp_start);
+	}
+	return 0;
+}
+
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA
+ * 	measurement log.
+ * @image: - pointer to the kimage, to store the address and size of the 
+ *	 IMA measurement log.
+ * @load_addr: - the address where the IMA measurement log is stored.
+ * @size - size of the IMA measurement log.
+ * 
+ * Return: 0 on success, negative errno on error.
+ */
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      size_t size)
+{
+	image->arch.ima_buffer_addr = load_addr;
+	image->arch.ima_buffer_size = size;
+	return 0;
+}
+
+/**
+ * setup_ima_buffer - update the fdt to contain the ima mesasurement log
+ * @image: - pointer to the kimage, containing the address and size of
+ *	     the IMA measurement log.
+ * @fdt: - pointer to the fdt.
+ * @chosen_node: - node under which property is to be defined.
+ *  
+ * Return: 0 on success, negative errno on error.
+ */
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
+{
+	int ret;
+
+	remove_ima_buffer(fdt, chosen_node);
+
+	if (!image->arch.ima_buffer_size)
+		return 0;
+
+	ret = fdt_setprop_u64(fdt, chosen_node, "linux,ima-kexec-buffer",
+			      image->arch.ima_buffer_addr);
+	if (ret < 0)
+		return ret;
+
+	ret = fdt_setprop_u64(fdt, chosen_node, "linux,ima-kexec-buffer-end",
+			      image->arch.ima_buffer_addr +
+			      image->arch.ima_buffer_size);
+	if (ret < 0)
+		return ret;
+
+	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
+			      image->arch.ima_buffer_size);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index 58871333737a..de5452539c67 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -21,6 +21,7 @@
 #include <linux/types.h>
 #include <linux/vmalloc.h>
 #include <asm/byteorder.h>
+#include <asm/ima.h>
 
 /* relevant device tree properties */
 #define FDT_PROP_INITRD_START	"linux,initrd-start"
@@ -85,6 +86,11 @@ static int setup_dtb(struct kimage *image,
 			goto out;
 	}
 
+	/* add ima measuremnet log buffer */
+	ret = setup_ima_buffer(image, dtb, off);
+	if (ret)
+		goto out;
+
 	/* add kaslr-seed */
 	ret = fdt_delprop(dtb, off, FDT_PROP_KASLR_SEED);
 	if  (ret == -FDT_ERR_NOTFOUND)
-- 
2.17.1

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Mimi Zohar]

Hi Prahkar,

On Fri, 2019-09-13 at 15:50 -0700, Prakhar Srivastava wrote:
> During kexec_file_load, carrying forward the ima measurement log allows
> a verifying party to get the entire runtime event log since the last
> full reboot since that is when PCRs were last reset.
> 
> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>

Missing is the Changelog between versions.  (v1 has now been posted 3
times.)

There's a number of trailing whitespaces, blanks before tabs, and
other warnings.  Please use scripts/checkpatch.pl before posting
patches.


> ---
>  arch/arm64/Kconfig                     |   7 +
>  arch/arm64/include/asm/ima.h           |  29 ++++
>  arch/arm64/include/asm/kexec.h         |   5 +
>  arch/arm64/kernel/Makefile             |   3 +-
>  arch/arm64/kernel/ima_kexec.c          | 213 +++++++++++++++++++++++++
>  arch/arm64/kernel/machine_kexec_file.c |   6 +
>  6 files changed, 262 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/include/asm/ima.h
>  create mode 100644 arch/arm64/kernel/ima_kexec.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 3adcec05b1f6..f39b12dbf9e8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
>  	  verification for the corresponding kernel image type being
>  	  loaded in order for this to work.
>  
> +config HAVE_IMA_KEXEC
> +	bool "Carry over IMA measurement log during kexec_file_load() syscall"
> +	depends on KEXEC_FILE
> +	help
> +	  Select this option to carry over IMA measurement log during
> +	  kexec_file_load.
> +
>  config KEXEC_IMAGE_VERIFY_SIG
>  	bool "Enable Image signature verification support"
>  	default y
> diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
> new file mode 100644
> index 000000000000..e23cee84729f
> --- /dev/null
> +++ b/arch/arm64/include/asm/ima.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM64_IMA_H
> +#define _ASM_ARM64_IMA_H
> +
> +struct kimage;
> +
> +int ima_get_kexec_buffer(void **addr, size_t *size);
> +int ima_free_kexec_buffer(void);
> +
> +#ifdef CONFIG_IMA
> +void remove_ima_buffer(void *fdt, int chosen_node);
> +#else
> +static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
> +#endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> +			      size_t size);
> +
> +int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
> +#else
> +static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
> +				   int chosen_node)
> +{
> +	remove_ima_buffer(fdt, chosen_node);
> +	return 0;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> +#endif /* _ASM_ARM64_IMA_H */
> diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
> index 12a561a54128..e8d2412066e7 100644
> --- a/arch/arm64/include/asm/kexec.h
> +++ b/arch/arm64/include/asm/kexec.h
> @@ -96,6 +96,11 @@ static inline void crash_post_resume(void) {}
>  struct kimage_arch {
>  	void *dtb;
>  	unsigned long dtb_mem;
> +
> +#ifdef CONFIG_IMA_KEXEC
> +	phys_addr_t ima_buffer_addr;
> +	size_t ima_buffer_size;
> +#endif
>  };
>  
>  extern const struct kexec_file_ops kexec_image_ops;
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 478491f07b4f..580238f2e9a7 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -55,7 +55,8 @@ obj-$(CONFIG_RANDOMIZE_BASE)		+= kaslr.o
>  obj-$(CONFIG_HIBERNATION)		+= hibernate.o hibernate-asm.o
>  obj-$(CONFIG_KEXEC_CORE)		+= machine_kexec.o relocate_kernel.o	\
>  					   cpu-reset.o
> -obj-$(CONFIG_KEXEC_FILE)		+= machine_kexec_file.o kexec_image.o
> +obj-$(CONFIG_KEXEC_FILE)		+= machine_kexec_file.o kexec_image.o	\
> +					   ima_kexec.o

The kernel may be built with/without IMA enabled or without IMA_KEXEC
support.  Here you're requiring both IMA and IMA_KEXEC to be
configured.  Please refer to the powerpc example.


>  obj-$(CONFIG_ARM64_RELOC_TEST)		+= arm64-reloc-test.o
>  arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
>  obj-$(CONFIG_CRASH_DUMP)		+= crash_dump.o
> diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
> new file mode 100644
> index 000000000000..b14326d541f3
> --- /dev/null
> +++ b/arch/arm64/kernel/ima_kexec.c
> @@ -0,0 +1,213 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 Microsoft Corporation.
> + *
> + * Authors:
> + * Prakhar Srivastava <prsriva@linux.microsoft.com>
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/kexec.h>
> +#include <linux/of.h>
> +#include <linux/memblock.h>
> +#include <linux/libfdt.h>
> +
> +

Extraneous blank line.

> +/**
> + * delete_fdt_mem_rsv - delete memory reservation with given address and size
> + * @fdt - pointer to the fdt.
> + * @start - start address of the memory.
> + * @size - number of cells to be deletd.
> + * 
> + * Return: 0 on success, or negative errno on error.
> + */
> +int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
> +{
> +	int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
> +
> +	for (i = 0; i < num_rsvs; i++) {
> +		uint64_t rsv_start, rsv_size;
> +
> +		ret = fdt_get_mem_rsv(fdt, i, &rsv_start, &rsv_size);
> +		if (ret) {
> +			pr_err("Malformed device tree\n");
> +			return -EINVAL;
> +		}
> +
> +		if (rsv_start == start && rsv_size == size) {
> +			ret = fdt_del_mem_rsv(fdt, i);
> +			if (ret) {
> +				pr_err("Error deleting device tree reservation\n");
> +				return -EINVAL;
> +			}
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +/**
> + * remove_ima_buffer - remove the IMA buffer property and reservation
> + * @fdt - pointer the fdt.
> + * @chosen_node - node under which property can be found.
> + * 
> + * The IMA measurement buffer is either read by now and freeed or a kexec call
> + * needs to replace the ima measurement buffer, clear the property and memory
> + * reservation.
> + */
> +void remove_ima_buffer(void *fdt, int chosen_node)
> +{
> +	int ret, len;
> +	const void *prop;
> +	uint64_t tmp_start, tmp_end;
> +
> +	prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
> +	if (prop) {
> +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
> +
> +		prop = fdt_getprop(fdt, chosen_node,
> +				   "linux,ima-kexec-buffer-end", &len);
> +		if (!prop)
> +			return;
> +
> +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
> +
> +		ret = delete_fdt_mem_rsv(fdt, tmp_start, tmp_end - tmp_start);
> +
> +		if (ret == 0)
> +			pr_debug("Removed old IMA buffer reservation.\n");
> +		else if (ret != -ENOENT)
> +			return;
> +
> +		fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
> +		fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer-end");
> +	}
> +}
> +
> +/**
> + * ima_get_kexec_buffer - get IMA buffer from the previous kernel
> + * @addr:	On successful return, set to point to the buffer contents.
> + * @size:	On successful return, set to the buffer size.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> +	int len;
> +	const void *prop;
> +	uint64_t tmp_start, tmp_end;
> +
> +	prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
> +
> +	prop = of_get_property(of_chosen, "linux,ima-kexec-buffer-end", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
> +
> +	*addr = __va(tmp_start);
> +	*size = tmp_end - tmp_start;
> +
> +	return 0;
> +}
> +
> +/**
> + * ima_free_kexec_buffer - free memory used by the IMA buffer
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int ima_free_kexec_buffer(void)
> +{
> +	int ret;
> +	void *propStart, *propEnd;

Is there a reason for using mixed case variables?  Why does the
variable need to be prefixed with "prop"?  Wouldn't ima_start/_stop,
 buffer_start/_stop, or even buf_start/_stop be fine?

> +	uint64_t tmp_start, tmp_end;
> +
> +	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
> +				     NULL);
> +	if (propStart) {
> +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
> +		ret = of_remove_property(of_chosen, propStart);
> +		if (!ret) {
> +			return ret;
> +		}
> +
> +		propEnd = of_find_property(of_chosen,
> +					   "linux,ima-kexec-buffer-end", NULL);
> +		if (!propEnd) {
> +			return -EINVAL;
> +		}
> +
> +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
> +
> +		ret = of_remove_property(of_chosen, propEnd);
> +		if (!ret) {
> +			return ret;
> +		}

There seems to be quite a bit of code duplication in this function and
in ima_get_kexec_buffer().  It could probably be cleaned up with some
refactoring.

Mimi

> +
> +		return memblock_free(tmp_start, tmp_end - tmp_start);
> +	}
> +	return 0;
> +}
> +
> +#ifdef CONFIG_IMA_KEXEC
> +/**
> + * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA
> + * 	measurement log.
> + * @image: - pointer to the kimage, to store the address and size of the 
> + *	 IMA measurement log.
> + * @load_addr: - the address where the IMA measurement log is stored.
> + * @size - size of the IMA measurement log.
> + * 
> + * Return: 0 on success, negative errno on error.
> + */
> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> +			      size_t size)
> +{
> +	image->arch.ima_buffer_addr = load_addr;
> +	image->arch.ima_buffer_size = size;
> +	return 0;
> +}
> +
> +/**
> + * setup_ima_buffer - update the fdt to contain the ima mesasurement log
> + * @image: - pointer to the kimage, containing the address and size of
> + *	     the IMA measurement log.
> + * @fdt: - pointer to the fdt.
> + * @chosen_node: - node under which property is to be defined.
> + *  
> + * Return: 0 on success, negative errno on error.
> + */
> +int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
> +{
> +	int ret;
> +
> +	remove_ima_buffer(fdt, chosen_node);
> +
> +	if (!image->arch.ima_buffer_size)
> +		return 0;
> +
> +	ret = fdt_setprop_u64(fdt, chosen_node, "linux,ima-kexec-buffer",
> +			      image->arch.ima_buffer_addr);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = fdt_setprop_u64(fdt, chosen_node, "linux,ima-kexec-buffer-end",
> +			      image->arch.ima_buffer_addr +
> +			      image->arch.ima_buffer_size);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
> +			      image->arch.ima_buffer_size);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> index 58871333737a..de5452539c67 100644
> --- a/arch/arm64/kernel/machine_kexec_file.c
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -21,6 +21,7 @@
>  #include <linux/types.h>
>  #include <linux/vmalloc.h>
>  #include <asm/byteorder.h>
> +#include <asm/ima.h>
>  
>  /* relevant device tree properties */
>  #define FDT_PROP_INITRD_START	"linux,initrd-start"
> @@ -85,6 +86,11 @@ static int setup_dtb(struct kimage *image,
>  			goto out;
>  	}
>  
> +	/* add ima measuremnet log buffer */
> +	ret = setup_ima_buffer(image, dtb, off);
> +	if (ret)
> +		goto out;
> +
>  	/* add kaslr-seed */
>  	ret = fdt_delprop(dtb, off, FDT_PROP_KASLR_SEED);
>  	if  (ret == -FDT_ERR_NOTFOUND)

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Mimi Zohar]

On Wed, 2019-09-18 at 10:15 -0400, Mimi Zohar wrote:

> > +	uint64_t tmp_start, tmp_end;
> > +
> > +	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
> > +				     NULL);
> > +	if (propStart) {
> > +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
> > +		ret = of_remove_property(of_chosen, propStart);
> > +		if (!ret) {
> > +			return ret;
> > +		}
> > +
> > +		propEnd = of_find_property(of_chosen,
> > +					   "linux,ima-kexec-buffer-end", NULL);
> > +		if (!propEnd) {
> > +			return -EINVAL;
> > +		}
> > +
> > +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
> > +
> > +		ret = of_remove_property(of_chosen, propEnd);
> > +		if (!ret) {
> > +			return ret;
> > +		}
> 
> There seems to be quite a bit of code duplication in this function and
> in ima_get_kexec_buffer().  It could probably be cleaned up with some
> refactoring.

Sorry, my mistake.  One calls of_get_property(), while the other calls
of_find_property().

Mimi

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Thiago Jung Bauermann]

Mimi Zohar <zohar@linux.ibm.com> writes:

> On Wed, 2019-09-18 at 10:15 -0400, Mimi Zohar wrote:
>
>> > +	uint64_t tmp_start, tmp_end;
>> > +
>> > +	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
>> > +				     NULL);
>> > +	if (propStart) {
>> > +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
>> > +		ret = of_remove_property(of_chosen, propStart);
>> > +		if (!ret) {
>> > +			return ret;
>> > +		}
>> > +
>> > +		propEnd = of_find_property(of_chosen,
>> > +					   "linux,ima-kexec-buffer-end", NULL);
>> > +		if (!propEnd) {
>> > +			return -EINVAL;
>> > +		}
>> > +
>> > +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
>> > +
>> > +		ret = of_remove_property(of_chosen, propEnd);
>> > +		if (!ret) {
>> > +			return ret;
>> > +		}
>> 
>> There seems to be quite a bit of code duplication in this function and
>> in ima_get_kexec_buffer().  It could probably be cleaned up with some
>> refactoring.
>
> Sorry, my mistake.  One calls of_get_property(), while the other calls
> of_find_property().

of_get_property() is a thin wrapper around of_find_property(), so if
that's the only difference I think they can still be merged.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Prakhar Srivastava]

On 9/18/2019 8:59 PM, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
>> On Wed, 2019-09-18 at 10:15 -0400, Mimi Zohar wrote:
>>
>>>> +	uint64_t tmp_start, tmp_end;
>>>> +
>>>> +	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
>>>> +				     NULL);
>>>> +	if (propStart) {
>>>> +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
>>>> +		ret = of_remove_property(of_chosen, propStart);
>>>> +		if (!ret) {
>>>> +			return ret;
>>>> +		}
>>>> +
>>>> +		propEnd = of_find_property(of_chosen,
>>>> +					   "linux,ima-kexec-buffer-end", NULL);
>>>> +		if (!propEnd) {
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
>>>> +
>>>> +		ret = of_remove_property(of_chosen, propEnd);
>>>> +		if (!ret) {
>>>> +			return ret;
>>>> +		}
>>> There seems to be quite a bit of code duplication in this function and
>>> in ima_get_kexec_buffer().  It could probably be cleaned up with some
>>> refactoring.
>> Sorry, my mistake.  One calls of_get_property(), while the other calls
>> of_find_property().
> of_get_property() is a thin wrapper around of_find_property(), so if
> that's the only difference I think they can still be merged.

I will move to using of_get_property and see if i can reduce the code 
further.

Also address other comments by Mimi in my next iteration.

Thanks,

Prakhar Srivastava

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Thiago Jung Bauermann]

Hello Prakhar,

Prakhar Srivastava <prsriva@linux.microsoft.com> writes:

> During kexec_file_load, carrying forward the ima measurement log allows
> a verifying party to get the entire runtime event log since the last
> full reboot since that is when PCRs were last reset.
>
> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> ---
>  arch/arm64/Kconfig                     |   7 +
>  arch/arm64/include/asm/ima.h           |  29 ++++
>  arch/arm64/include/asm/kexec.h         |   5 +
>  arch/arm64/kernel/Makefile             |   3 +-
>  arch/arm64/kernel/ima_kexec.c          | 213 +++++++++++++++++++++++++
>  arch/arm64/kernel/machine_kexec_file.c |   6 +
>  6 files changed, 262 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/include/asm/ima.h
>  create mode 100644 arch/arm64/kernel/ima_kexec.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 3adcec05b1f6..f39b12dbf9e8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
>  	  verification for the corresponding kernel image type being
>  	  loaded in order for this to work.
>
> +config HAVE_IMA_KEXEC
> +	bool "Carry over IMA measurement log during kexec_file_load() syscall"
> +	depends on KEXEC_FILE
> +	help
> +	  Select this option to carry over IMA measurement log during
> +	  kexec_file_load.
> +
>  config KEXEC_IMAGE_VERIFY_SIG
>  	bool "Enable Image signature verification support"
>  	default y

This is not right. As it stands, HAVE_IMA_KEXEC is essentially a synonym
for IMA_KEXEC.

It's not meant to be user-visible in the config process. Instead, it's
meant to be selected by the arch Kconfig (probably by the ARM64 config
symbol) to signal to IMA's Kconfig that it can offer the IMA_KEXEC
option.

I also mentioned in my previous review that config HAVE_IMA_KEXEC should
be defined in arch/Kconfig, not separately in both arch/arm64/Kconfig
and arch/powerpc/Kconfig.

> diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
> new file mode 100644
> index 000000000000..e23cee84729f
> --- /dev/null
> +++ b/arch/arm64/include/asm/ima.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM64_IMA_H
> +#define _ASM_ARM64_IMA_H
> +
> +struct kimage;
> +
> +int ima_get_kexec_buffer(void **addr, size_t *size);
> +int ima_free_kexec_buffer(void);
> +
> +#ifdef CONFIG_IMA
> +void remove_ima_buffer(void *fdt, int chosen_node);
> +#else
> +static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
> +#endif

I mentioned in my previous review that remove_ima_buffer() should exist
even if CONFIG_IMA isn't set. Did you arrive at a different conclusion?

> +
> +#ifdef CONFIG_IMA_KEXEC
> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
> +			      size_t size);
> +
> +int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
> +#else
> +static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
> +				   int chosen_node)
> +{
> +	remove_ima_buffer(fdt, chosen_node);
> +	return 0;
> +}
> +#endif /* CONFIG_IMA_KEXEC */
> +#endif /* _ASM_ARM64_IMA_H */

> diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
> new file mode 100644
> index 000000000000..b14326d541f3
> --- /dev/null
> +++ b/arch/arm64/kernel/ima_kexec.c

In the previous patch, you took the powerpc file and made a few
modifications to fit your needs. This file is now somewhat different
than the powerpc version, but I don't understand to what purpose. It's
not different in any significant way.

Based on review comments from your previous patch, I was expecting to
see code from the powerpc file moved to an arch-independent part of the
the kernel and possibly adapted so that both arm64 and powerpc could use
it. Can you explain why you chose this approach instead? What is the
advantage of having superficially different but basically equivalent
code in the two architectures?

Actually, there's one change that is significant: instead of a single
linux,ima-kexec-buffer property holding the start address and size of
the buffer, ARM64 is now using two properties (linux,ima-kexec-buffer
and linux,ima-kexec-buffer-end) for the start and end addresses. In my
opinion, unless there's a good reason for it Linux should be consistent
accross architectures when possible.

--
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[prsriva]

On 9/19/19 8:07 PM, Thiago Jung Bauermann wrote:
> Hello Prakhar,
>
> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
>
>> During kexec_file_load, carrying forward the ima measurement log allows
>> a verifying party to get the entire runtime event log since the last
>> full reboot since that is when PCRs were last reset.
>>
>> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
>> ---
>>   arch/arm64/Kconfig                     |   7 +
>>   arch/arm64/include/asm/ima.h           |  29 ++++
>>   arch/arm64/include/asm/kexec.h         |   5 +
>>   arch/arm64/kernel/Makefile             |   3 +-
>>   arch/arm64/kernel/ima_kexec.c          | 213 +++++++++++++++++++++++++
>>   arch/arm64/kernel/machine_kexec_file.c |   6 +
>>   6 files changed, 262 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm64/include/asm/ima.h
>>   create mode 100644 arch/arm64/kernel/ima_kexec.c
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 3adcec05b1f6..f39b12dbf9e8 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
>>   	  verification for the corresponding kernel image type being
>>   	  loaded in order for this to work.
>>
>> +config HAVE_IMA_KEXEC
>> +	bool "Carry over IMA measurement log during kexec_file_load() syscall"
>> +	depends on KEXEC_FILE
>> +	help
>> +	  Select this option to carry over IMA measurement log during
>> +	  kexec_file_load.
>> +
>>   config KEXEC_IMAGE_VERIFY_SIG
>>   	bool "Enable Image signature verification support"
>>   	default y
> This is not right. As it stands, HAVE_IMA_KEXEC is essentially a synonym
> for IMA_KEXEC.
>
> It's not meant to be user-visible in the config process. Instead, it's
> meant to be selected by the arch Kconfig (probably by the ARM64 config
> symbol) to signal to IMA's Kconfig that it can offer the IMA_KEXEC
> option.
>
> I also mentioned in my previous review that config HAVE_IMA_KEXEC should
> be defined in arch/Kconfig, not separately in both arch/arm64/Kconfig
> and arch/powerpc/Kconfig.

I see the entry exists in arch/Kconfig and is overwritten.
I will remove entries both from powerpc and arm64.

How do i cross-compile for powerpc?

>
>> diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
>> new file mode 100644
>> index 000000000000..e23cee84729f
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/ima.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_ARM64_IMA_H
>> +#define _ASM_ARM64_IMA_H
>> +
>> +struct kimage;
>> +
>> +int ima_get_kexec_buffer(void **addr, size_t *size);
>> +int ima_free_kexec_buffer(void);
>> +
>> +#ifdef CONFIG_IMA
>> +void remove_ima_buffer(void *fdt, int chosen_node);
>> +#else
>> +static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
>> +#endif
> I mentioned in my previous review that remove_ima_buffer() should exist
> even if CONFIG_IMA isn't set. Did you arrive at a different conclusion?

I made the needed changed in makefile, missed removing the

configs here. Thanks for pointing this out.

>> +
>> +#ifdef CONFIG_IMA_KEXEC
>> +int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
>> +			      size_t size);
>> +
>> +int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
>> +#else
>> +static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
>> +				   int chosen_node)
>> +{
>> +	remove_ima_buffer(fdt, chosen_node);
>> +	return 0;
>> +}
>> +#endif /* CONFIG_IMA_KEXEC */
>> +#endif /* _ASM_ARM64_IMA_H */
>> diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
>> new file mode 100644
>> index 000000000000..b14326d541f3
>> --- /dev/null
>> +++ b/arch/arm64/kernel/ima_kexec.c
> In the previous patch, you took the powerpc file and made a few
> modifications to fit your needs. This file is now somewhat different
> than the powerpc version, but I don't understand to what purpose. It's
> not different in any significant way.
>
> Based on review comments from your previous patch, I was expecting to
> see code from the powerpc file moved to an arch-independent part of the
> the kernel and possibly adapted so that both arm64 and powerpc could use
> it. Can you explain why you chose this approach instead? What is the
> advantage of having superficially different but basically equivalent
> code in the two architectures?
>
> Actually, there's one change that is significant: instead of a single
> linux,ima-kexec-buffer property holding the start address and size of
> the buffer, ARM64 is now using two properties (linux,ima-kexec-buffer
> and linux,ima-kexec-buffer-end) for the start and end addresses. In my
> opinion, unless there's a good reason for it Linux should be consistent
> accross architectures when possible.
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[Thiago Jung Bauermann]

Hello,

prsriva <prsriva@linux.microsoft.com> writes:

> On 9/19/19 8:07 PM, Thiago Jung Bauermann wrote:
>> Hello Prakhar,
>>
>> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 3adcec05b1f6..f39b12dbf9e8 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -976,6 +976,13 @@ config KEXEC_VERIFY_SIG
>>>   	  verification for the corresponding kernel image type being
>>>   	  loaded in order for this to work.
>>>
>>> +config HAVE_IMA_KEXEC
>>> +	bool "Carry over IMA measurement log during kexec_file_load() syscall"
>>> +	depends on KEXEC_FILE
>>> +	help
>>> +	  Select this option to carry over IMA measurement log during
>>> +	  kexec_file_load.
>>> +
>>>   config KEXEC_IMAGE_VERIFY_SIG
>>>   	bool "Enable Image signature verification support"
>>>   	default y
>> This is not right. As it stands, HAVE_IMA_KEXEC is essentially a synonym
>> for IMA_KEXEC.
>>
>> It's not meant to be user-visible in the config process. Instead, it's
>> meant to be selected by the arch Kconfig (probably by the ARM64 config
>> symbol) to signal to IMA's Kconfig that it can offer the IMA_KEXEC
>> option.
>>
>> I also mentioned in my previous review that config HAVE_IMA_KEXEC should
>> be defined in arch/Kconfig, not separately in both arch/arm64/Kconfig
>> and arch/powerpc/Kconfig.
>
> I see the entry exists in arch/Kconfig and is overwritten.
> I will remove entries both from powerpc and arm64.
>
> How do i cross-compile for powerpc?

There are some instructions here:

https://github.com/linuxppc/wiki/wiki/Building-powerpc-kernels

>>> diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
>>> new file mode 100644
>>> index 000000000000..e23cee84729f
>>> --- /dev/null
>>> +++ b/arch/arm64/include/asm/ima.h
>>> @@ -0,0 +1,29 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +#ifndef _ASM_ARM64_IMA_H
>>> +#define _ASM_ARM64_IMA_H
>>> +
>>> +struct kimage;
>>> +
>>> +int ima_get_kexec_buffer(void **addr, size_t *size);
>>> +int ima_free_kexec_buffer(void);
>>> +
>>> +#ifdef CONFIG_IMA
>>> +void remove_ima_buffer(void *fdt, int chosen_node);
>>> +#else
>>> +static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
>>> +#endif
>> I mentioned in my previous review that remove_ima_buffer() should exist
>> even if CONFIG_IMA isn't set. Did you arrive at a different conclusion?
>
> I made the needed changed in makefile, missed removing the
>
> configs here. Thanks for pointing this out.

Thanks.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load

[prsriva]

On 9/19/19 8:07 PM, Thiago Jung Bauermann wrote:
> 
> Hello Prakhar,
> 
> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
> 
>> During kexec_file_load, carrying forward the ima measurement log allows
>> a verifying party to get the entire runtime event log since the last
>> full reboot since that is when PCRs were last reset.

<snip>

> In the previous patch, you took the powerpc file and made a few
> modifications to fit your needs. This file is now somewhat different
> than the powerpc version, but I don't understand to what purpose. It's
> not different in any significant way.
> 
> Based on review comments from your previous patch, I was expecting to
> see code from the powerpc file moved to an arch-independent part of the
> the kernel and possibly adapted so that both arm64 and powerpc could use
> it. Can you explain why you chose this approach instead? What is the
> advantage of having superficially different but basically equivalent
> code in the two architectures?
> 
> Actually, there's one change that is significant: instead of a single
> linux,ima-kexec-buffer property holding the start address and size of
> the buffer, ARM64 is now using two properties (linux,ima-kexec-buffer
> and linux,ima-kexec-buffer-end) for the start and end addresses. In my
> opinion, unless there's a good reason for it Linux should be consistent
> accross architectures when possible.
> 
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center

I looked at the of_ drivers are it became apparent that the driver calls
were already available for consumption. Adding ima specific code will be
same as adding wrapper code for any other property. Which is true for
all properties, effectively setting the property name and pass through
for other parameters.

I still like to move both implementations to a arch independent code 
path, i could not convince my self that of_*ima is probably the place, 
but if that's the best place?, then i will go ahead and make that change 
as well.

Regarding using two properties, it just seemed more consistent how the
properties(start-end) are being used in the kexec, and hides the inner 
details for the cell structures, thats all.

Its just the placement of the wrapper functions, but once thats done
both archs will call the same.

Thanks,
Prakhar Srivastava

> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>