Re: [PATCH v17 05/10] powerpc: Move ima buffer fields to struct kimage
From: Rob Herring <robh@kernel.org>
Date: 2021-02-10 17:21:27
Also in:
linux-arm-kernel, linux-devicetree, linux-integrity, lkml
On Tue, Feb 09, 2021 at 10:21:55AM -0800, Lakshmi Ramasubramanian wrote:
quoted hunk ↗ jump to hunk
The fields ima_buffer_addr and ima_buffer_size in "struct kimage_arch" for powerpc are used to carry forward the IMA measurement list across kexec system call. These fields are not architecture specific, but are currently limited to powerpc. arch_ima_add_kexec_buffer() defined in "arch/powerpc/kexec/ima.c" sets ima_buffer_addr and ima_buffer_size for the kexec system call. This function does not have architecture specific code, but is currently limited to powerpc. Move ima_buffer_addr and ima_buffer_size to "struct kimage". Rename arch_ima_add_kexec_buffer() to of_ima_add_kexec_buffer() and move it in drivers/of/kexec.c. Co-developed-by: Prakhar Srivastava <redacted> Signed-off-by: Prakhar Srivastava <redacted> Signed-off-by: Lakshmi Ramasubramanian <redacted> Suggested-by: Will Deacon <will@kernel.org> --- arch/powerpc/include/asm/ima.h | 3 --- arch/powerpc/include/asm/kexec.h | 5 ----- arch/powerpc/kexec/ima.c | 29 ++++++----------------------- drivers/of/kexec.c | 23 +++++++++++++++++++++++ include/linux/kexec.h | 3 +++ include/linux/of.h | 5 +++++ security/integrity/ima/ima_kexec.c | 3 ++- 7 files changed, 39 insertions(+), 32 deletions(-)diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h index ead488cf3981..51f64fd06c19 100644 --- a/arch/powerpc/include/asm/ima.h +++ b/arch/powerpc/include/asm/ima.h@@ -14,9 +14,6 @@ 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,diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h index bdd0ddb9ac4d..ecf88533d6b4 100644 --- a/arch/powerpc/include/asm/kexec.h +++ b/arch/powerpc/include/asm/kexec.h@@ -112,11 +112,6 @@ struct kimage_arch { unsigned long elf_headers_sz; void *elf_headers; void *fdt; - -#ifdef CONFIG_IMA_KEXEC - phys_addr_t ima_buffer_addr; - size_t ima_buffer_size; -#endif }; char *setup_kdump_cmdline(struct kimage *image, char *cmdline,diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c index 720e50e490b6..ed38125e2f87 100644 --- a/arch/powerpc/kexec/ima.c +++ b/arch/powerpc/kexec/ima.c@@ -128,23 +128,6 @@ void remove_ima_buffer(void *fdt, int chosen_node) } #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) {@@ -180,7 +163,7 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node) u8 value[16]; remove_ima_buffer(fdt, chosen_node); - if (!image->arch.ima_buffer_size) + if (!image->ima_buffer_size) return 0; ret = get_addr_size_cells(&addr_cells, &size_cells);@@ -192,11 +175,11 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node) if (entry_size > sizeof(value)) return -EINVAL; - ret = write_number(value, image->arch.ima_buffer_addr, addr_cells); + ret = write_number(value, image->ima_buffer_addr, addr_cells); if (ret) return ret; - ret = write_number(value + 4 * addr_cells, image->arch.ima_buffer_size, + ret = write_number(value + 4 * addr_cells, image->ima_buffer_size, size_cells); if (ret) return ret;@@ -206,13 +189,13 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node) if (ret < 0) return -EINVAL; - ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr, - image->arch.ima_buffer_size); + ret = fdt_add_mem_rsv(fdt, image->ima_buffer_addr, + image->ima_buffer_size); if (ret) return -EINVAL; pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n", - image->arch.ima_buffer_addr, image->arch.ima_buffer_size); + image->ima_buffer_addr, image->ima_buffer_size); return 0; }diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c index 469e09613cdd..9f33d215b9f2 100644 --- a/drivers/of/kexec.c +++ b/drivers/of/kexec.c@@ -63,6 +63,29 @@ static int fdt_find_and_del_mem_rsv(void *fdt, unsigned long start, unsigned lon return -ENOENT; } +#ifdef CONFIG_IMA_KEXEC +/** + * of_ima_add_kexec_buffer - Add IMA buffer for next kernel + * + * @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 + * + * Use this function to pass on the IMA buffer information to + * the next kernel across kexec system call. + * + * Return: 0 on success, negative errno on error. + */ +int of_ima_add_kexec_buffer(struct kimage *image, + unsigned long load_addr, size_t size) +{ + image->ima_buffer_addr = load_addr; + image->ima_buffer_size = size; +
There's nothing DT specific about this function, so this is the wrong place for it. I would just remove it and directly set the members. Rob