Re: [PATCH v12 2/4] powerpc: Move arch independent ima kexec functions to drivers/of/kexec.c

From: Lakshmi Ramasubramanian
Date: Fri Dec 18 2020 - 01:26:31 EST


On 12/17/20 2:01 PM, Rob Herring wrote:


[...]

+#ifdef CONFIG_IMA_KEXEC
+/**
+ * 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.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+ size_t size)

This should be a static inline in asm/kexec.h.

arch_ima_add_kexec_buffer() is identical for powerpc and arm64.
Would it be better to "static inline" this function in "of.h" instead of
duplicating it in "asm/kexec.h" for powerpc and arm64?

No, think about what it is specific to and place it there. It has
nothing to do with DT really. All it is is a wrapper to access the
struct members in kimage_arch. So it belongs where they are declared.
Now perhaps ima_buffer_addr and ima_buffer_size shouldn't be arch
specific, but that's a separate issue.


Since "struct kimage" definition is not available in "asm/kexec.h", defining arch_ima_add_kexec_buffer() in this header file results in the following build error:

./arch/powerpc/include/asm/kexec.h: In function 'arch_ima_add_kexec_buffer':
./arch/powerpc/include/asm/kexec.h:139:7: error: 'struct kimage' has no member named 'arch'
139 | image->arch.ima_buffer_addr = load_addr;

I think it would be appropriate to make arch_ima_add_kexec_buffer() a static inline function in "security/integrity/ima/ima_kexec.c" - the only file where this function is used.

This will also enable sharing this function for powerpc and arm64 architectures.

thanks,
-lakshmi