2020-12-14 19:27:47

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v11 0/8] Carry forward IMA measurement log on kexec on ARM64

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 already supports carrying forward the IMA measurement log on
kexec. This patch set adds support for carrying forward the IMA
measurement log on kexec on ARM64.

This patch set moves the platform independent code defined for powerpc
such that it can be reused for other platforms as well. A chosen node
"linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
the address and the size of the memory reserved to carry
the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU.
I would like help from the community for testing this change on powerpc.
Thanks.

This patch set is based on
commit a29a64445089 ("powerpc: Use common of_kexec_setup_new_fdt()")
in https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git "dt/kexec" branch.

Changelog:

v11
- Rebased the changes on the kexec code refactoring done by
Rob Herring in his "dt/kexec" branch
- Removed "extern" keyword in function declarations
- Removed unnecessary header files included in C files
- Updated patch descriptions per Thiago's comments

v10
- Moved delete_fdt_mem_rsv(), remove_ima_buffer(),
get_ima_kexec_buffer, and get_root_addr_size_cells()
to drivers/of/kexec.c
- Moved arch_ima_add_kexec_buffer() to
security/integrity/ima/ima_kexec.c
- Conditionally define IMA buffer fields in struct kimage_arch

v9
- Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
- Defined a new function get_ima_kexec_buffer() in
drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
- Changed remove_ima_kexec_buffer() to the original function name
remove_ima_buffer()
- Moved remove_ima_buffer() to drivers/of/ima_kexec.c
- Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
to security/integrity/ima/ima_kexec.c

v8:
- Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
delete_fdt_mem_rsv() to drivers/of/fdt.c
- Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
back to security/integrity/ima/ima_kexec.c

v7:
- Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
this function definition to kernel.
- Moved delete_fdt_mem_rsv() definition to kernel
- Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
a new file namely ima_kexec_fdt.c in IMA

v6:
- Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device
tree and also its corresponding memory reservation in the currently
running kernel.
- Moved the function remove_ima_buffer() defined for powerpc to IMA
and renamed the function to ima_remove_kexec_buffer(). Also, moved
delete_fdt_mem_rsv() from powerpc to IMA.

v5:
- Merged get_addr_size_cells() and do_get_kexec_buffer() into a single
function when moving the arch independent code from powerpc to IMA
- Reverted the change to use FDT functions in powerpc code and added
back the original code in get_addr_size_cells() and
do_get_kexec_buffer() for powerpc.
- Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
the IMA log buffer during kexec.
- Fixed the warning reported by kernel test bot for ARM64
arch_ima_add_kexec_buffer() - moved this function to a new file
namely arch/arm64/kernel/ima_kexec.c

v4:
- Submitting the patch series on behalf of the original author
Prakhar Srivastava <[email protected]>
- Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
libfdt.h so that it can be shared by multiple platforms.

v3:
Breakup patches further into separate patches.
- Refactoring non architecture specific code out of powerpc
- Update powerpc related code to use fdt functions
- Update IMA buffer read related code to use of functions
- Add support to store the memory information of the IMA
measurement logs to be carried forward.
- Update the property strings to align with documented nodes
https://github.com/devicetree-org/dt-schema/pull/46

v2:
Break patches into separate patches.
- Powerpc related Refactoring
- Updating the docuemntation for chosen node
- Updating arm64 to support IMA buffer pass

v1:
Refactoring carrying over IMA measuremnet logs over Kexec. This patch
moves the non-architecture specific code out of powerpc and adds to
security/ima.(Suggested by Thiago)
Add Documentation regarding the ima-kexec-buffer node in the chosen
node documentation

v0:
Add a layer of abstraction to use the memory reserved by device tree
for ima buffer pass.
Add support for ima buffer pass using reserved memory for arm64 kexec.
Update the arch sepcific code path in kexec file load to store the
ima buffer in the reserved memory. The same reserved memory is read
on kexec or cold boot.

Lakshmi Ramasubramanian (8):
powerpc: Fix compiler warnings and errors
powerpc: Move ima buffer functions to drivers/of/kexec.c
powerpc: Use ima kexec node functions
powerpc: Move remove_ima_buffer() to drivers/of/kexec.c
powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to
ima
powerpc: Move arch_ima_add_kexec_buffer to ima
arm64: Free DTB buffer if fdt_open_into() fails
arm64: Add IMA log information in kimage used for kexec

arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/ima.h | 24 +++++
arch/arm64/include/asm/kexec.h | 5 +
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/ima.c | 49 +++++++++
arch/arm64/kernel/machine_kexec_file.c | 12 ++-
arch/powerpc/include/asm/ima.h | 14 +--
arch/powerpc/include/asm/kexec.h | 1 -
arch/powerpc/kexec/Makefile | 7 +-
arch/powerpc/kexec/file_load.c | 32 ------
arch/powerpc/kexec/ima.c | 138 +------------------------
drivers/of/kexec.c | 94 +++++++++++++++++
include/linux/of.h | 5 +-
security/integrity/ima/ima.h | 4 -
security/integrity/ima/ima_kexec.c | 73 +++++++++++++
15 files changed, 266 insertions(+), 194 deletions(-)
create mode 100644 arch/arm64/include/asm/ima.h
create mode 100644 arch/arm64/kernel/ima.c

--
2.29.2


2020-12-14 19:28:24

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v11 1/8] powerpc: Fix compiler warnings and errors

The function prototype for the functions defined in ima.c for powerpc
are given in the header file ima.h. But this header file is not
included in ima.c resulting in compilation errors such as given below.

arch/powerpc/kexec/ima.c:56:5: error: no previous prototype for 'ima_get_kexec_buffer' [-Werror=missing-prototypes]
56 | int ima_get_kexec_buffer(void **addr, size_t *size)
| ^~~~~~~~~~~~~~~~~~~~

The function parameters for remove_ima_buffer() and
arch_ima_add_kexec_buffer() are not described in the function header
resulting in warnings such as given below.

arch/powerpc/kexec/ima.c:111: warning: Function parameter or member 'fdt' not described in 'remove_ima_buffer'

Include ima.h in ima.c for powerpc. Describe the function parameters for
remove_ima_buffer() and arch_ima_add_kexec_buffer().

Co-developed-by: Prakhar Srivastava <[email protected]>
Signed-off-by: Prakhar Srivastava <[email protected]>
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Reviewed-by: Thiago Jung Bauermann <[email protected]>
---
arch/powerpc/kexec/ima.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
index 720e50e490b6..a36c39db4b1a 100644
--- a/arch/powerpc/kexec/ima.c
+++ b/arch/powerpc/kexec/ima.c
@@ -11,6 +11,7 @@
#include <linux/of.h>
#include <linux/memblock.h>
#include <linux/libfdt.h>
+#include <asm/ima.h>

static int get_addr_size_cells(int *addr_cells, int *size_cells)
{
@@ -103,6 +104,9 @@ int ima_free_kexec_buffer(void)
/**
* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
*
+ * @fdt: Flattened Device Tree to update
+ * @chosen_node: Offset to the chosen node in the device tree
+ *
* The IMA measurement buffer is of no use to a subsequent kernel, so we always
* remove it from the device tree.
*/
@@ -131,6 +135,10 @@ void remove_ima_buffer(void *fdt, int chosen_node)
/**
* arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
*
+ * @image: kimage struct to set IMA buffer data
+ * @load_addr: Starting address where IMA buffer is loaded at
+ * @size: Number of bytes in the IMA buffer
+ *
* Architectures should use this function to pass on the IMA buffer
* information to the next kernel.
*
--
2.29.2

2020-12-14 19:28:24

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: [PATCH v11 2/8] powerpc: Move ima buffer functions to drivers/of/kexec.c

The functions do_get_kexec_buffer() and get_addr_size_cells(),
defined in arch/powerpc/kexec/ima.c, retrieve the address and size
of the given property from the device tree blob. These functions do
not have architecture specific code, but are currently limited to
powerpc. do_get_kexec_buffer() correctly handles a device tree property
that is a child node of the root node, but not anything other than
the immediate root child nodes.

Move architecture independent functions get_ima_kexec_buffer() and
get_root_addr_size_cells() to "drivers/of/kexec.c". These functions
retrieve the chosen node "linux,ima-kexec-buffer" from the device tree,
and return the address and size of the buffer used for carrying forward
the IMA measurement log across kexec system call.

Co-developed-by: Prakhar Srivastava <[email protected]>
Signed-off-by: Prakhar Srivastava <[email protected]>
Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
---
drivers/of/kexec.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of.h | 3 ++
2 files changed, 71 insertions(+)

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 66787be081fe..9af5371340b1 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -30,6 +30,10 @@
/**
* fdt_find_and_del_mem_rsv - delete memory reservation with given address and size
*
+ * @fdt: Flattened Device Tree to update
+ * @start: Starting address of the reservation to delete
+ * @size: Size of the reservation to delete
+ *
* Return: 0 on success, or negative errno on error.
*/
static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned long size)
@@ -226,3 +230,67 @@ int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,

return 0;
}
+
+/**
+ * get_root_addr_size_cells - Get address and size of root node
+ *
+ * @addr_cells: Return address of the root node
+ * @size_cells: Return size of the root node
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+int get_root_addr_size_cells(int *addr_cells, int *size_cells)
+{
+ struct device_node *root;
+
+ root = of_find_node_by_path("/");
+ if (!root)
+ return -EINVAL;
+
+ *addr_cells = of_n_addr_cells(root);
+ *size_cells = of_n_size_cells(root);
+
+ of_node_put(root);
+
+ return 0;
+}
+
+/**
+ * get_ima_kexec_buffer - Get address and size of IMA kexec buffer
+ *
+ * @fdt: Flattened Device Tree
+ * @chosen_node: Offset of chosen node in the FDT
+ * @addr: Return address of the node
+ * @size: Return size of the node
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+int get_ima_kexec_buffer(void *fdt, int chosen_node,
+ unsigned long *addr, size_t *size)
+{
+ const void *prop;
+ int addr_cells, size_cells, prop_len;
+ int rc;
+
+ rc = get_root_addr_size_cells(&addr_cells, &size_cells);
+ if (rc)
+ return rc;
+
+ if (fdt)
+ prop = fdt_getprop(fdt, chosen_node,
+ "linux,ima-kexec-buffer", &prop_len);
+ else
+ prop = of_get_property(of_chosen,
+ "linux,ima-kexec-buffer", &prop_len);
+
+ if (!prop)
+ return -ENOENT;
+
+ if (prop_len < 4 * (addr_cells + size_cells))
+ return -EINVAL;
+
+ *addr = of_read_number(prop, addr_cells);
+ *size = of_read_number(prop + 4 * addr_cells, size_cells);
+
+ return 0;
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index 3375f5295875..fb2ef274135d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -562,6 +562,9 @@ struct kimage;
int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
unsigned long initrd_load_addr, unsigned long initrd_len,
const char *cmdline);
+int get_root_addr_size_cells(int *addr_cells, int *size_cells);
+int get_ima_kexec_buffer(void *fdt, int chosen_node,
+ unsigned long *addr, size_t *size);

#else /* CONFIG_OF */

--
2.29.2

2020-12-15 18:19:52

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v11 2/8] powerpc: Move ima buffer functions to drivers/of/kexec.c

On Mon, Dec 14, 2020 at 11:18:48AM -0800, Lakshmi Ramasubramanian wrote:
> The functions do_get_kexec_buffer() and get_addr_size_cells(),
> defined in arch/powerpc/kexec/ima.c, retrieve the address and size
> of the given property from the device tree blob. These functions do
> not have architecture specific code, but are currently limited to
> powerpc. do_get_kexec_buffer() correctly handles a device tree property
> that is a child node of the root node, but not anything other than
> the immediate root child nodes.
>
> Move architecture independent functions get_ima_kexec_buffer() and
> get_root_addr_size_cells() to "drivers/of/kexec.c". These functions
> retrieve the chosen node "linux,ima-kexec-buffer" from the device tree,
> and return the address and size of the buffer used for carrying forward
> the IMA measurement log across kexec system call.
>
> Co-developed-by: Prakhar Srivastava <[email protected]>
> Signed-off-by: Prakhar Srivastava <[email protected]>
> Signed-off-by: Lakshmi Ramasubramanian <[email protected]>
> ---
> drivers/of/kexec.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h | 3 ++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 66787be081fe..9af5371340b1 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -30,6 +30,10 @@
> /**
> * fdt_find_and_del_mem_rsv - delete memory reservation with given address and size
> *
> + * @fdt: Flattened Device Tree to update
> + * @start: Starting address of the reservation to delete
> + * @size: Size of the reservation to delete
> + *

This belongs in the patch adding fdt_find_and_del_mem_rsv.

> * Return: 0 on success, or negative errno on error.
> */
> static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned long size)
> @@ -226,3 +230,67 @@ int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
>
> return 0;
> }
> +
> +/**
> + * get_root_addr_size_cells - Get address and size of root node
> + *
> + * @addr_cells: Return address of the root node
> + * @size_cells: Return size of the root node
> + *
> + * Return: 0 on success, or negative errno on error.
> + */
> +int get_root_addr_size_cells(int *addr_cells, int *size_cells)
> +{
> + struct device_node *root;
> +
> + root = of_find_node_by_path("/");
> + if (!root)
> + return -EINVAL;
> +
> + *addr_cells = of_n_addr_cells(root);
> + *size_cells = of_n_size_cells(root);
> +
> + of_node_put(root);
> +
> + return 0;
> +}
> +
> +/**
> + * get_ima_kexec_buffer - Get address and size of IMA kexec buffer
> + *
> + * @fdt: Flattened Device Tree
> + * @chosen_node: Offset of chosen node in the FDT
> + * @addr: Return address of the node
> + * @size: Return size of the node
> + *
> + * Return: 0 on success, or negative errno on error.
> + */
> +int get_ima_kexec_buffer(void *fdt, int chosen_node,
> + unsigned long *addr, size_t *size)
> +{
> + const void *prop;
> + int addr_cells, size_cells, prop_len;
> + int rc;
> +
> + rc = get_root_addr_size_cells(&addr_cells, &size_cells);
> + if (rc)
> + return rc;
> +
> + if (fdt)
> + prop = fdt_getprop(fdt, chosen_node,
> + "linux,ima-kexec-buffer", &prop_len);
> + else
> + prop = of_get_property(of_chosen,
> + "linux,ima-kexec-buffer", &prop_len);

This is an odd structure. The DT APIs are generally of_foo() for
unflattened tree and of_flat_foo() for FDT.

> +
> + if (!prop)
> + return -ENOENT;
> +
> + if (prop_len < 4 * (addr_cells + size_cells))
> + return -EINVAL;
> +
> + *addr = of_read_number(prop, addr_cells);
> + *size = of_read_number(prop + 4 * addr_cells, size_cells);
> +
> + return 0;
> +}
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 3375f5295875..fb2ef274135d 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -562,6 +562,9 @@ struct kimage;
> int of_kexec_setup_new_fdt(const struct kimage *image, void *fdt,
> unsigned long initrd_load_addr, unsigned long initrd_len,
> const char *cmdline);
> +int get_root_addr_size_cells(int *addr_cells, int *size_cells);
> +int get_ima_kexec_buffer(void *fdt, int chosen_node,
> + unsigned long *addr, size_t *size);

The whole point of moving code out of arch code was to avoid all these
cross tree calls.

I was expecting the next step to be moving setup_ima_buffer() call into
of_kexec_setup_new_fdt with all the code in powerpc/kexec/ima.c moved
here. Then the end result to enable on arm64 is just selecting
HAVE_IMA_KEXEC and adding fields to kimage_arch.

Rob