Hello Lakshmi, Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> writes: > On 10/20/20 8:17 PM, Mimi Zohar wrote: >> On Tue, 2020-10-20 at 19:25 -0700, Lakshmi Ramasubramanian wrote: >>> On 10/20/20 1:00 PM, Mimi Zohar wrote: >>>> Hi Lakshmi, >>>> >>>> On Wed, 2020-09-30 at 13:59 -0700, Lakshmi Ramasubramanian wrote: >>>>> The functions remove_ima_buffer() and delete_fdt_mem_rsv() that handle >>>>> carrying forward the IMA measurement logs on kexec for powerpc do not >>>>> have architecture specific code, but they are currently defined for >>>>> powerpc only. >>>>> >>>>> remove_ima_buffer() and delete_fdt_mem_rsv() are used to remove >>>>> the IMA log entry from the device tree and free the memory reserved >>>>> for the log. These functions need to be defined even if the current >>>>> kernel does not support carrying forward IMA log across kexec since >>>>> the previous kernel could have supported that and therefore the current >>>>> kernel needs to free the allocation. >>>>> >>>>> Rename remove_ima_buffer() to remove_ima_kexec_buffer(). >>>>> Define remove_ima_kexec_buffer() and delete_fdt_mem_rsv() in kernel. >>>>> A later patch in this series will use these functions to free >>>>> the allocation, if any, made by the previous kernel for ARM64. >>>>> >>>>> Define FDT_PROP_IMA_KEXEC_BUFFER for the chosen node, namely >>>>> "linux,ima-kexec-buffer", that is added to the DTB to hold >>>>> the address and the size of the memory reserved to carry >>>>> the IMA measurement log. >>>> >>>>> Co-developed-by: Prakhar Srivastava <prsriva@xxxxxxxxxxxxxxxxxxx> >>>>> Signed-off-by: Prakhar Srivastava <prsriva@xxxxxxxxxxxxxxxxxxx> >>>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx> >>>>> Reported-by: kernel test robot <lkp@xxxxxxxxx> error: implicit declaration of function 'delete_fdt_mem_rsv' [-Werror,-Wimplicit-function-declaration] >>>> >>>> Much better! This version limits unnecessarily changing the existing >>>> code to adding a couple of debugging statements, but that looks to be >>>> about it. >>> Yes Mimi - that's correct. >>> >>>> >>>> Based on Chester Lin's "ima_arch" support for arm64 discussion, the IMA generic >>>> EFI support will be defined in ima/ima-efi.c. Similarly, I think it would make sense to put the generic device tree support in ima/ima_kexec_fdt.c or ima/ima_fdt.c, as opposed to kernel/. (Refer to my comments on 2/4 about the new file named ima_kexec_fdt.c.) >>> >>> The functions remove_ima_kexec_buffer() and delete_fdt_mem_rsv(), which >>> are defined in kernel/ima_kexec.c and kernel/kexec_file_fdt.c >>> respectively, are needed even when CONFIG_IMA is not defined. These >>> functions need to be called by the current kernel to free the ima kexec >>> buffer resources allocated by the previous kernel. This is the reason, >>> these functions are defined under "kernel" instead of >>> "security/integrity/ima". >>> >>> If there is a better location to move the above C files, please let me >>> know. I'll move them. >> Freeing the previous kernel measurement list is currently called from >> ima_load_kexec_buffer(), only after the measurement list has been >> restored. The only other time the memory is freed is when the >> allocated memory size isn't sufficient to hold the measurement list, >> which could happen if there is a delay between loading and executing >> the kexec. >> > > There are two "free" operations we need to perform with respect to ima buffer on > kexec: > > 1, The ima_free_kexec_buffer() called from ima_load_kexec_buffer() - the one you > have stated above. > > Here we remove the "ima buffer" node from the "OF" tree and free the memory > pages that were allocated for the measurement list. > > This one is already present in ima and there's no change in that in my patches. > > 2, The other one is remove_ima_kexec_buffer() called from setup_ima_buffer() > defined in "arch/powerpc/kexec/ima.c" > > This function removes the "ima buffer" node from the "FDT" and also frees the > physical memory reserved for the "ima measurement list" by the previous kernel. > > This "free" operation needs to be performed even if the current kernel does not > support IMA kexec since the previous kernel could have passed the IMA > measurement list (in FDT and reserved physical memory). > > For this reason, remove_ima_kexec_buffer() cannot be defined in "ima" but some > other place which will be built even if ima is not enabled. I chose to define > this function in "kernel" since that is guaranteed to be always built. > > thanks, > -lakshmi That is true. I believe a more fitting place for these functions is drivers/of/fdt.c rather than these new files in kernel/. Both CONFIG_PPC and CONFIG_ARM64 select CONFIG_OF and CONFIG_OF_FLATTREE (indirectly, via CONFIG_OF_EARLY_FLATTREE) so they will both build that file. -- Thiago Jung Bauermann IBM Linux Technology Center
