Carry ima measurement log for arm64 via kexec_file_load
Subsystems (3)
cover Carry ima measurement log for arm64 via kexec_file_load
The patch adds support for arm64 to carry ima measurement log to the next soft boot session triggered via kexec_file_load.
- Top of Linux 5.3-rc6
Currently during kexec the kernel file signatures are validated prior to actual load, the information(PE/ima signature) is not carried to the next session.
This lead to loss of information.
This patch addresses the same by carrying forward the ima measurement log to the next kexec'ed session. This just allows a verifying party to get the entire runtime event log since the last full reboot since that is when PCRs were last reset.
The code is in most part same as powerpc, i want to get feedback as to how/correct way to refactor the code so that cross architecture partial helpers can be put in a common place.
Prakhar Srivastava (1):
Carry ima measurement log for arm64 via kexec_file_load
arch/arm64/Kconfig | 7 + arch/arm64/include/asm/ima.h | 31 ++++ arch/arm64/include/asm/kexec.h | 4 + arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/ima_kexec.c | 219 +++++++++++++++++++++++++ arch/arm64/kernel/machine_kexec_file.c | 39 +++++ 6 files changed, 301 insertions(+) create mode 100644 arch/arm64/include/asm/ima.h create mode 100644 arch/arm64/kernel/ima_kexec.c
Signature · 1 line
2.17.1
RFC PATCH 1/1 Carry ima measurement log for arm64 via kexec_file_load
Carry ima measurement log for arm64 via kexec_file_load. add support to kexec_file_load to pass the ima measurement log
This patch adds entry for the ima measurement log in the dtb which is then used in the kexec'ed session to fetch the segment and then load the ima measurement log.
--- arch/arm64/Kconfig | 7 + arch/arm64/include/asm/ima.h | 31 ++++ arch/arm64/include/asm/kexec.h | 4 + arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/ima_kexec.c | 219 +++++++++++++++++++++++++ arch/arm64/kernel/machine_kexec_file.c | 39 +++++ 6 files changed, 301 insertions(+) create mode 100644 arch/arm64/include/asm/ima.h create mode 100644 arch/arm64/kernel/ima_kexec.c
arch/arm64/Kconfig +7 -0
@@ -964,6 +964,13 @@ config KEXEC_FILE for kernel and initramfs as opposed to list of segments as accepted by previous system call. +config HAVE_IMA_KEXEC + bool "enable arch specific ima buffer pass" + depends on KEXEC_FILE + help + This adds support to carry ima log to the next kernel in case + of kexec_file_load + config KEXEC_VERIFY_SIG bool "Verify kernel signature during kexec_file_load() syscall" depends on KEXEC_FILE
arch/arm64/include/asm/ima.h new file +31 -0
@@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_ARM64_IMA_H +#define _ASM_ARM64_IMA_H + +#define FDT_PROP_KEXEC_BUFFER "linux,ima-kexec-buffer" + +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 */
arch/arm64/include/asm/kexec.h +4 -0
@@ -96,6 +96,8 @@ static inline void crash_post_resume(void) {}struct kimage_arch {void *dtb;unsigned long dtb_mem; +phys_addr_t ima_buffer_addr; +size_t ima_buffer_size; };extern const struct kexec_file_ops kexec_image_ops;
@@ -107,6 +109,8 @@ extern int load_other_segments(struct kimage *image, unsigned long kernel_load_addr, unsigned long kernel_size, char *initrd, unsigned long initrd_len, char *cmdline); +extern int delete_fdt_mem_rsv (void *fdt,unsigned long start, +unsigned long size);#endif #endif /* __ASSEMBLY__ */
arch/arm64/kernel/Makefile +1 -0
@@ -63,6 +63,7 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_SSBD) += ssbd.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o +obj-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-y += vdso/ probes/ obj-$(CONFIG_COMPAT_VDSO) += vdso32/
arch/arm64/kernel/ima_kexec.c new file +219 -0
219 lines not on this page · raw message
arch/arm64/kernel/machine_kexec_file.c +39 -0
@@ -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_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 )
@@ -114,6 +120,39 @@ static int setup_dtb(struct kimage *image,*/ #define DTB_EXTRA_SPACE 0x1000 + +/** +* delete_fdt_mem_rsv - delete memory reservation with given address and size +* +* 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 ; +} +static int create_dtb (struct kimage *image,unsigned long initrd_load_addr,unsigned long initrd_len,char *cmdline,void **dtb)
Signature · 1 line
2.17.1
Why is linux-arm-msm list CCed on this topic?
Quoting Prakhar Srivastava (2019-08-29 13:05:32)
Quoted 2 lines
Carry ima measurement log for arm64 via
kexec_file_load. add support tokexec_file_loadto pass the ima measurement log
These first two sentences look sort of odd for a commit text.
Quoted 2 lines
This patch adds entry for the ima measurement log in the
Don't use 'this patch' in commit text. It's in the wrong voice.
Quoted 9 lines
dtb which is then used in the kexec'ed session to fetch the segment and then load the ima measurement log.
Signed-off-by Prakhar Srivastava <[email protected]>arch/arm64/Kconfig +0
-0
arch/arm64/Kconfig config KEXEC_FILE · line 971
Quoted 8 lines
arch/arm64/Kconfig +5
-0@@ -964,6 +964,13 @@ config KEXEC_FILE for kernel and initramfs as opposed to list of segments as accepted by previous system call. +config HAVE_IMA_KEXEC + bool "enable arch specific ima buffer pass" + depends on KEXEC_FILE + help + This adds support to carry ima log to the next kernel in case
Should ima be all caps here?
arch/arm64/Kconfig config KEXEC_FILE · line 972
Quoted 1 line
arch/arm64/Kconfig +1
-0+ of kexec_file_load
Add a full-stop?
Quoted 10 lines
arch/arm64/Kconfig +1
-0+ config KEXEC_VERIFY_SIG bool "Verify kernel signature during kexec_file_load() syscall" depends on KEXEC_FILEarch/arm64/include/asm/ima.h new file +0
-0
arch/arm64/include/asm/ima.h line 5
Quoted 5 lines
arch/arm64/include/asm/ima.h new file +5
-0@@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_ARM64_IMA_H +#define _ASM_ARM64_IMA_H + +#define FDT_PROP_KEXEC_BUFFER "linux,ima-kexec-buffer"
Is this documented somewhere in the DT bindings?
Quoted 31 lines
arch/arm64/include/asm/ima.h new file +26
-0+ +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 */ arch/arm64/include/asm/kexec.h +0
-0
arch/arm64/include/asm/kexec.h static inline void crash_post_resume(void) {} · line 100
Quoted 5 lines
arch/arm64/include/asm/kexec.h +2
-0@@ -96,6 +96,8 @@ static inline void crash_post_resume(void) {}struct kimage_arch {void *dtb;unsigned long dtb_mem; +phys_addr_t ima_buffer_addr; +size_t ima_buffer_size;
Should this be in an ifdef?
Quoted 18 lines
arch/arm64/include/asm/kexec.h +2
-0};extern const struct kexec_file_ops kexec_image_ops;@@ -107,6 +109,8 @@ extern int load_other_segments(struct kimage *image, unsigned long kernel_load_addr, unsigned long kernel_size, char *initrd, unsigned long initrd_len, char *cmdline); +extern int delete_fdt_mem_rsv (void *fdt,unsigned long start, +unsigned long size);#endif #endif /* __ASSEMBLY__ */ arch/arm64/kernel/ima_kexec.c new file +0
-0
arch/arm64/kernel/ima_kexec.c line 10
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0@@ -0,0 +1,219 @@ +/* +* Copyright (C) 2016 IBM Corporation +* +* Authors: +* Thiago Jung Bauermann <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version.
Please use a SPDX tag instead of this boiler plate.
Quoted 134 lines
arch/arm64/kernel/ima_kexec.c new file +134
-0+*/ + +#include <linux/slab.h> +#include <linux/kexec.h> +#include <linux/of.h> +#include <linux/memblock.h> +#include <linux/libfdt.h> +#include <asm/kexec.h> +#include <asm/ima.h> + +static int get_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 ; +} + +static int do_get_kexec_buffer (const void *prop,int len,unsigned long *addr, +size_t *size) +{ + +int ret, addr_cells, size_cells; + + ret =get_addr_size_cells (&addr_cells, &size_cells); +if (ret) +return ret; + +if (len <4 * (addr_cells + size_cells)) +return -ENOENT ; + + *addr =of_read_number (prop, addr_cells); + *size =of_read_number (prop +4 * addr_cells, size_cells); + +return 0 ; +} + +/** +* 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 ret, len; +unsigned long tmp_addr; +size_t tmp_size; +const void *prop; + + prop =of_get_property (of_chosen,FDT_PROP_KEXEC_BUFFER , &len); +if (!prop) +return -ENOENT ; + + ret =do_get_kexec_buffer (prop, len, &tmp_addr, &tmp_size); +if (ret) +return ret; + + *addr =__va (tmp_addr); + *size = tmp_size; +return 0 ; +} + +/** +* ima_free_kexec_buffer - free memory used by the IMA buffer +*/ +int ima_free_kexec_buffer (void ) +{ +int ret; +unsigned long addr; +size_t size; +struct property *prop; + + prop =of_find_property (of_chosen,FDT_PROP_KEXEC_BUFFER ,NULL ); +if (!prop) +return -ENOENT ; + + ret =do_get_kexec_buffer (prop->value, prop->length, &addr, &size); +if (ret) +return ret; + + ret =of_remove_property (of_chosen, prop); +if (ret) +return ret; + +return memblock_free (addr, size); +} + +#ifdef CONFIG_IMA +/** +* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt +* +* The IMA measurement buffer is of no use to a subsequent kernel, so we always +* remove it from the device tree. +*/ +void remove_ima_buffer (void *fdt,int chosen_node) +{ +int ret, len; +unsigned long addr; +size_t size; +const void *prop; + + prop =fdt_getprop (fdt, chosen_node,FDT_PROP_KEXEC_BUFFER , &len); +if (!prop) +return ; + + ret =do_get_kexec_buffer (prop, len, &addr, &size); +fdt_delprop (fdt, chosen_node,FDT_PROP_KEXEC_BUFFER ); +if (ret) +return ; + + ret =delete_fdt_mem_rsv (fdt, addr, size); +if (!ret) +pr_debug ("Removed old IMA buffer reservation.\n" ); +} +#endif /* CONFIG_IMA */ + +#ifdef CONFIG_IMA_KEXEC +/** +* arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer +* +* Architectures should use this function to pass on the IMA buffer +* information to the next kernel. +* +* Return: 0 on success, negative errno on error.
arch/arm64/kernel/ima_kexec.c line 154
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+*/ +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 ; +} + +static int write_number (void *p,u64 value,int cells)
Maybe this should be an of_write_number() API exposed by the DT parsing code?
Quoted 6 lines
arch/arm64/kernel/ima_kexec.c new file +6
-0+{ +if (cells ==1 ) { +u32 tmp; + +if (value >U32_MAX ) +return -EINVAL ;
arch/arm64/kernel/ima_kexec.c line 170
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+ + tmp =cpu_to_be32 (value); +memcpy (p, &tmp,sizeof (tmp)); + }else if (cells ==2 ) { +u64 tmp; + + tmp =cpu_to_be64 (value); +memcpy (p, &tmp,sizeof (tmp)); + }else +return -EINVAL ;
Put braces around this else please.
arch/arm64/kernel/ima_kexec.c line 178
Quoted 8 lines
arch/arm64/kernel/ima_kexec.c new file +8
-0+return 0 ; +} + +/** +* setup_ima_buffer - add IMA buffer information to the fdt +* @image: kexec image being loaded. +* @dtb: Flattened device tree for the next kernel. +* @chosen_node: Offset to the chosen node.
Why capitalize Flattened and Offset?
Quoted 66 lines
arch/arm64/kernel/ima_kexec.c new file +41
-0+* +* Return: 0 on success, or negative errno on error. +*/ +int setup_ima_buffer (const struct kimage *image,void *dtb,int chosen_node) +{ +int ret, addr_cells, size_cells, entry_size; +u8 value[16 ]; + +remove_ima_buffer (dtb, chosen_node); + + ret =get_addr_size_cells (&addr_cells, &size_cells); +if (ret) +return ret; + + entry_size =4 * (addr_cells + size_cells); + +if (entry_size >sizeof (value)) +return -EINVAL ; + + ret =write_number (value, image->arch.ima_buffer_addr, addr_cells); +if (ret) +return ret; + + ret =write_number (value +4 * addr_cells, image->arch.ima_buffer_size, + size_cells); +if (ret) +return ret; + + ret =fdt_setprop (dtb, chosen_node,FDT_PROP_KEXEC_BUFFER , value, + entry_size); +if (ret <0 ) +return -EINVAL ; + + ret =fdt_add_mem_rsv (dtb, image->segment[0 ].mem, + image->segment[0 ].memsz); +if (ret) +return -EINVAL ; + +return 0 ; +} +#endif /* CONFIG_IMA_KEXEC */ arch/arm64/kernel/machine_kexec_file.c +6
-0@@ -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_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 )
arch/arm64/kernel/machine_kexec_file.c static int setup_dtb(struct kimage *image, · line 126
Quoted 7 lines
arch/arm64/kernel/machine_kexec_file.c +4
-0@@ -114,6 +120,39 @@ static int setup_dtb(struct kimage *image,*/ #define DTB_EXTRA_SPACE 0x1000 + +/** +* delete_fdt_mem_rsv - delete memory reservation with given address and size +*
Can you document the arguments too?
arch/arm64/kernel/machine_kexec_file.c static int setup_dtb(struct kimage *image, · line 138
Quoted 12 lines
arch/arm64/kernel/machine_kexec_file.c +12
-0+* 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" );
Please drop the full-stop on this printk.
arch/arm64/kernel/machine_kexec_file.c static int setup_dtb(struct kimage *image, · line 145
Quoted 7 lines
arch/arm64/kernel/machine_kexec_file.c +7
-0+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" );
Same comment.
arch/arm64/kernel/machine_kexec_file.c static int setup_dtb(struct kimage *image, · line 154
Quoted 9 lines
arch/arm64/kernel/machine_kexec_file.c +9
-0+return -EINVAL ; + } + +return 0 ; + } + } + +return -ENOENT ; +}
A lot of this code looks DT generic. Can it be moved out of the arch layer to drivers/of/?
Quoted 2 lines · Stephen Boyd
A lot of this code looks DT generic. Can it be moved out of the arch layer to
drivers/of/?
Yes, if this code could be in drivers/of/ it would be great! Perhaps the
DT generic functions could go in drivers/of/fdt.c, and ones dealing with IMA nodes/properties could go in drivers/of/fdt_ima.c?
--
Thiago Jung Bauermann
IBM Linux Technology Center
Hello Prakhar,
Answering this part from the cover letter:
Quoted 3 lines
The code is in most part same as powerpc, i want to get feedback as to how/correct way to refactor the code so that cross architecture partial helpers can be put in a common place.
That's a great idea. If it could go to drivers/of/ as Stephen Boyd mentioned in the other email that would be great.
More comments below.
Quoted 27 lines · Prakhar Srivastava
Carry ima measurement log for arm64 via
kexec_file_load. add support tokexec_file_loadto pass the ima measurement logThis patch adds entry for the ima measurement log in the dtb which is then used in the kexec'ed session to fetch the segment and then load the ima measurement log.
Signed-off-by Prakhar Srivastava <[email protected]>--- arch/arm64/Kconfig | 7 + arch/arm64/include/asm/ima.h | 31 ++++ arch/arm64/include/asm/kexec.h | 4 + arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/ima_kexec.c | 219 +++++++++++++++++++++++++ arch/arm64/kernel/machine_kexec_file.c | 39 +++++ 6 files changed, 301 insertions(+) create mode 100644 arch/arm64/include/asm/ima.h create mode 100644 arch/arm64/kernel/ima_kexec.carch/arm64/Kconfig +0
-0@@ -964,6 +964,13 @@ config KEXEC_FILE for kernel and initramfs as opposed to list of segments as accepted by previous system call.
arch/arm64/Kconfig config KEXEC_FILE · line 976
Quoted 10 lines
arch/arm64/Kconfig +7
-0+config HAVE_IMA_KEXEC + bool "enable arch specific ima buffer pass" + depends on KEXEC_FILE + help + This adds support to carry ima log to the next kernel in case + of kexec_file_load + config KEXEC_VERIFY_SIG bool "Verify kernel signature during kexec_file_load() syscall" depends on KEXEC_FILE
This Kconfig should be defined in arch/Kconfig so that both arm64 and powerpc can select it.
Quoted 19 lines
arch/arm64/kernel/Makefile +1
-0@@ -63,6 +63,7 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_SSBD) += ssbd.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o +obj-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o obj-y += vdso/ probes/ obj-$(CONFIG_COMPAT_VDSO) += vdso32/arch/arm64/kernel/ima_kexec.c new file +1
-0@@ -0,0 +1,219 @@ +/*
arch/arm64/kernel/ima_kexec.c line 11
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+* Copyright (C) 2016 IBM Corporation +* +* Authors: +* Thiago Jung Bauermann <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +*/
The powerpc file was updated to use the SPDX tag and remove the paragraph above. Please update your file to match.
Quoted 18 lines
arch/arm64/kernel/ima_kexec.c new file +18
-0+#include <linux/slab.h> +#include <linux/kexec.h> +#include <linux/of.h> +#include <linux/memblock.h> +#include <linux/libfdt.h> +#include <asm/kexec.h> +#include <asm/ima.h> + +static int get_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);
arch/arm64/kernel/ima_kexec.c line 40
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+ +of_node_put (root); + +return 0 ; +} + +static int do_get_kexec_buffer (const void *prop,int len,unsigned long *addr, +size_t *size) +{ +
This spurious blank line only exists in the arm64 version. Should be removed.
Quoted 55 lines
arch/arm64/kernel/ima_kexec.c new file +55
-0+int ret, addr_cells, size_cells; + + ret =get_addr_size_cells (&addr_cells, &size_cells); +if (ret) +return ret; + +if (len <4 * (addr_cells + size_cells)) +return -ENOENT ; + + *addr =of_read_number (prop, addr_cells); + *size =of_read_number (prop +4 * addr_cells, size_cells); + +return 0 ; +} + +/** +* 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 ret, len; +unsigned long tmp_addr; +size_t tmp_size; +const void *prop; + + prop =of_get_property (of_chosen,FDT_PROP_KEXEC_BUFFER , &len); +if (!prop) +return -ENOENT ; + + ret =do_get_kexec_buffer (prop, len, &tmp_addr, &tmp_size); +if (ret) +return ret; + + *addr =__va (tmp_addr); + *size = tmp_size; +return 0 ; +} + +/** +* ima_free_kexec_buffer - free memory used by the IMA buffer +*/ +int ima_free_kexec_buffer (void ) +{ +int ret; +unsigned long addr; +size_t size; +struct property *prop; + + prop =of_find_property (of_chosen,FDT_PROP_KEXEC_BUFFER ,NULL ); +if (!prop) +return -ENOENT ;
arch/arm64/kernel/ima_kexec.c line 105
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+ + ret =do_get_kexec_buffer (prop->value, prop->length, &addr, &size); +if (ret) +return ret; + + ret =of_remove_property (of_chosen, prop); +if (ret) +return ret; + +return memblock_free (addr, size);
On the other hand you removed the spurious blank line that exists in powerpc. Thanks. :-)
arch/arm64/kernel/ima_kexec.c line 108
Quoted 3 lines
arch/arm64/kernel/ima_kexec.c new file +3
-0+} + +#ifdef CONFIG_IMA
remove_ima_buffer() should exist even if CONFIG_IMA isn't set. If I recall correctly, Mimi requested that kernels that are booted via kexec should remove the IMA buffer even if they won't use it.
Quoted 65 lines
arch/arm64/kernel/ima_kexec.c new file +65
-0+/** +* remove_ima_buffer - remove the IMA buffer property and reservation from @fdt +* +* The IMA measurement buffer is of no use to a subsequent kernel, so we always +* remove it from the device tree. +*/ +void remove_ima_buffer (void *fdt,int chosen_node) +{ +int ret, len; +unsigned long addr; +size_t size; +const void *prop; + + prop =fdt_getprop (fdt, chosen_node,FDT_PROP_KEXEC_BUFFER , &len); +if (!prop) +return ; + + ret =do_get_kexec_buffer (prop, len, &addr, &size); +fdt_delprop (fdt, chosen_node,FDT_PROP_KEXEC_BUFFER ); +if (ret) +return ; + + ret =delete_fdt_mem_rsv (fdt, addr, size); +if (!ret) +pr_debug ("Removed old IMA buffer reservation.\n" ); +} +#endif /* CONFIG_IMA */ + +#ifdef CONFIG_IMA_KEXEC +/** +* arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer +* +* Architectures should use this function to pass on the IMA buffer +* information to the next kernel. +* +* 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 ; +} + +static int write_number (void *p,u64 value,int cells) +{ +if (cells ==1 ) { +u32 tmp; + +if (value >U32_MAX ) +return -EINVAL ; + + tmp =cpu_to_be32 (value); +memcpy (p, &tmp,sizeof (tmp)); + }else if (cells ==2 ) { +u64 tmp; + + tmp =cpu_to_be64 (value); +memcpy (p, &tmp,sizeof (tmp)); + }else +return -EINVAL ; +return 0 ; +} +
arch/arm64/kernel/ima_kexec.c line 183
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+/** +* setup_ima_buffer - add IMA buffer information to the fdt +* @image: kexec image being loaded. +* @dtb: Flattened device tree for the next kernel. +* @chosen_node: Offset to the chosen node. +* +* Return: 0 on success, or negative errno on error. +*/ +int setup_ima_buffer (const struct kimage *image,void *dtb,int chosen_node) +{
Is there any particular reason to rename the fdt parameter to dtb?
arch/arm64/kernel/ima_kexec.c line 187
Quoted 4 lines
arch/arm64/kernel/ima_kexec.c new file +4
-0+int ret, addr_cells, size_cells, entry_size; +u8 value[16 ]; + +remove_ima_buffer (dtb, chosen_node);
In the powerpc version, there's an if here to return early if image->arch.ima_buffer_size == 0. Is there a reason why you removed it?
Quoted 16 lines
arch/arm64/kernel/ima_kexec.c new file +16
-0+ + ret =get_addr_size_cells (&addr_cells, &size_cells); +if (ret) +return ret; + + entry_size =4 * (addr_cells + size_cells); + +if (entry_size >sizeof (value)) +return -EINVAL ; + + ret =write_number (value, image->arch.ima_buffer_addr, addr_cells); +if (ret) +return ret; + + ret =write_number (value +4 * addr_cells, image->arch.ima_buffer_size, + size_cells);
arch/arm64/kernel/ima_kexec.c line 213
Quoted 10 lines
arch/arm64/kernel/ima_kexec.c new file +10
-0+if (ret) +return ret; + + ret =fdt_setprop (dtb, chosen_node,FDT_PROP_KEXEC_BUFFER , value, + entry_size); +if (ret <0 ) +return -EINVAL ; + + ret =fdt_add_mem_rsv (dtb, image->segment[0 ].mem, + image->segment[0 ].memsz);
powerpc uses image->arch.ima_buffer_addr and image->arch.ima_buffer_size here. Why do you use segment[0] above?
Quoted 56 lines
arch/arm64/kernel/ima_kexec.c new file +6
-0+if (ret) +return -EINVAL ; + + return 0; +} +#endif /* CONFIG_IMA_KEXEC */ arch/arm64/kernel/machine_kexec_file.c +28
-0@@ -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_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 )@@ -114,6 +120,39 @@ static int setup_dtb(struct kimage *image,*/ #define DTB_EXTRA_SPACE 0x1000 + +/** +* delete_fdt_mem_rsv - delete memory reservation with given address and size +* +* 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) {
arch/arm64/kernel/machine_kexec_file.c static int setup_dtb(struct kimage *image, · line 154
Quoted 10 lines
arch/arm64/kernel/machine_kexec_file.c +10
-0+pr_err ("Error deleting device tree reservation.\n" ); +return -EINVAL ; + } + +return 0 ; + } + } + +return -ENOENT ; +}
This functions is the same as the one in powerpc, and should be shared too.
--
Thiago Jung Bauermann
IBM Linux Technology Center
Quoted 7 lines · Thiago Jung Bauermann
Hello Prakhar,
Answering this part from the cover letter:
Quoted 3 lines
The code is in most part same as powerpc, i want to get feedback as to how/correct way to refactor the code so that cross architecture partial helpers can be put in a common place.
I started refactoring code to bring helpers under drivers/of, but
i soon reliazed the current implementation can be changed a bit
so that some of the additional functions can be sourced from
existing fdt_*/of_* functions since the fdt_ima was seeming to be
an overkill. I have done so in the V1 patch and also addressed
comments you have.
Hopefully its(v1) is a cleaner approach.
- Thanks for the review, and guidance.
Thanks,
Prakhar Srivastava
Quoted 7 lines
That's a great idea. If it could go to
drivers/of/as Stephen Boyd mentioned in the other email that would be great.More comments below.
-Addressed those in the v1 patch
Prakhar Srivastava <[email protected]> writes:
Mimi Zohar
maintainerHi Prakhar,
Quoted 17 lines · prsriva
Quoted 7 lines · Thiago Jung Bauermann
Hello Prakhar,
Answering this part from the cover letter:
Quoted 3 lines
The code is in most part same as powerpc, i want to get feedback as to how/correct way to refactor the code so that cross architecture partial helpers can be put in a common place.
I started refactoring code to bring helpers under
drivers/of, buti soon reliazed the current implementation can be changed a bit
so that some of the additional functions can be sourced from
existing fdt_*/of_* functions since the
fdt_imawas seeming to be
Quoted 7 lines
an overkill. I have done so in the V1 patch and also addressed
comments you have.
Hopefully its(v1) is a cleaner approach.
- Thanks for the review, and guidance.
"Carrying over the ima log during kexec_file_load" was originally posted on 5/10 and 5/31 without a cover letter. On 8/29 it was reposted as an RFC with a cover letter. The cover letter was v1, but the patch itself was not. In the future, please use the "git format- patch "-subject-prefix" option to add the version number to both the cover letter and the patches.
The comments you received were based on the 8/29 version. I haven't seen anything after that.
Mimi
Quoted 7 lines
Quoted 7 lines
That's a great idea. If it could go to
drivers/of/as Stephen Boyd mentioned in the other email that would be great.More comments below.
-Addressed those in the v1 patch
Prakhar Srivastava <[email protected]> writes:
Quoted 8 lines · Mimi Zohar
Hi Prakhar,
"Carrying over the ima log duringkexec_file_load" was originally posted on 5/10 and 5/31 without a cover letter. On 8/29 it was reposted as an RFC with a cover letter. The cover letter was v1, but the patch itself was not. In the future, please use the "git format- patch "-subject-prefix" option to add the version number to both the cover letter and the patches.
Quoted 4 lines
The comments you received were based on the 8/29 version. I haven't seen anything after that.
Mimi
Added the correct subject prefix, resent patches.
I resent the patches sent at 5/31, since i had no response.
v1 version addresses the comments in the RFC patch.
I have cc-ed you to the email, hopefully its not missed again.
Thankyou for pointing it out.
Thanks,
Prakhar Srivastava