Carry forward IMA measurement log on kexec on ARM64

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 <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> error: implicit declaration of function 'delete_fdt_mem_rsv' [-Werror,-Wimplicit-function-declaration]

Hi Lakshmi,

Quoted 21 lines · Lakshmi Ramasubramanian

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.

Quoted 4 lines

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> 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.

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.)

Quoted 25 lines · Mimi Zohar

Hi Lakshmi,

Quoted 21 lines · Lakshmi Ramasubramanian

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.

Quoted 8 lines

Quoted 4 lines

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> 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.

Quoted 3 lines

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.

Quoted 48 lines · Lakshmi Ramasubramanian

Quoted 25 lines · Mimi Zohar

Hi Lakshmi,

Quoted 21 lines · Lakshmi Ramasubramanian

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.

Quoted 8 lines

Quoted 4 lines

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> 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.

Quoted 3 lines

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".

Quoted 2 lines

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.

Quoted 49 lines · Mimi Zohar

Quoted 48 lines · Lakshmi Ramasubramanian

Quoted 25 lines · Mimi Zohar

Hi Lakshmi,

Quoted 21 lines · Lakshmi Ramasubramanian

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.

Quoted 8 lines

Quoted 4 lines

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> 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.

Quoted 3 lines

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".

Quoted 9 lines

Quoted 2 lines

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

Quoted 85 lines · Lakshmi Ramasubramanian

Quoted 49 lines · Mimi Zohar

Quoted 48 lines · Lakshmi Ramasubramanian

Quoted 25 lines · Mimi Zohar

Hi Lakshmi,

Quoted 21 lines · Lakshmi Ramasubramanian

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.

Quoted 8 lines

Quoted 4 lines

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Reported-by kernel test robot <[email protected]> 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.

Quoted 3 lines

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".

Quoted 9 lines

Quoted 2 lines

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.

Quoted 2 lines

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.

The functions ima_get_kexec_buffer() and ima_free_kexec_buffer(), 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.

Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to IMA subsystem. A later patch in this series will use these functions for carrying forward the IMA measurement log for ARM64.

With the above refactoring arch/powerpc/kexec/ima.c contains only functions used when CONFIG_IMA_KEXEC is enabled. Update Makefile in arch/powerpc/kexec to include arch/powerpc/kexec/ima.c only when CONFIG_IMA_KEXEC is enabled.

Move ima_dump_measurement_list() and ima_add_kexec_buffer() to a new file namely ima_kexec_fdt.c in IMA. Update security/integrity/ima/Makefile to include ima_kexec_fdt.c only when CONFIG_IMA_KEXEC is enabled.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Quoted 14 lines · Lakshmi Ramasubramanian

The functions ima_get_kexec_buffer() and ima_free_kexec_buffer(), 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.

Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to IMA subsystem. A later patch in this series will use these functions for carrying forward the IMA measurement log for ARM64.

With the above refactoring arch/powerpc/kexec/ima.c contains only functions used when CONFIG_IMA_KEXEC is enabled. Update Makefile in arch/powerpc/kexec to include arch/powerpc/kexec/ima.c only when CONFIG_IMA_KEXEC is enabled.

Quoted 8 lines

Move ima_dump_measurement_list() and ima_add_kexec_buffer() to a new file namely ima_kexec_fdt.c in IMA. Update security/integrity/ima/Makefile to include ima_kexec_fdt.c only when CONFIG_IMA_KEXEC is enabled.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

The existing support for carrying the IMA measurement list across kexec is limited to powerpc. This patch set is adding similar support for arm64, making as much of the existing code as generic as possible. However ima_dump_measurement_list() is already generic, but for some reason this patch moves it to ima_kexec_fdt.c. ima_kexec_fdt.c should be limited to device tree specific code.

This patch is probably doing the right thing, but the way the patch is formatted it replaces parts of a function with a different function. With the changes suggested above and in 1/4, the next version should be clearer.

Quoted 24 lines · Mimi Zohar

Quoted 14 lines · Lakshmi Ramasubramanian

The functions ima_get_kexec_buffer() and ima_free_kexec_buffer(), 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.

Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to IMA subsystem. A later patch in this series will use these functions for carrying forward the IMA measurement log for ARM64.

With the above refactoring arch/powerpc/kexec/ima.c contains only functions used when CONFIG_IMA_KEXEC is enabled. Update Makefile in arch/powerpc/kexec to include arch/powerpc/kexec/ima.c only when CONFIG_IMA_KEXEC is enabled.

Quoted 8 lines

Move ima_dump_measurement_list() and ima_add_kexec_buffer() to a new file namely ima_kexec_fdt.c in IMA. Update security/integrity/ima/Makefile to include ima_kexec_fdt.c only when CONFIG_IMA_KEXEC is enabled.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Quoted 6 lines

The existing support for carrying the IMA measurement list across kexec is limited to powerpc. This patch set is adding similar support for arm64, making as much of the existing code as generic as possible. However ima_dump_measurement_list() is already generic, but for some reason this patch moves it to ima_kexec_fdt.c. ima_kexec_fdt.c should be limited to device tree specific code.

I wanted to split the functions defined under CONFIG_HAVE_IMA_KEXEC and CONFIG_IMA_KEXEC to separate files so that we can get rid of #ifdef in C file and instead conditionally compile the C files (using Makefile).

ima_dump_measurement_list() need to be defined only when CONFIG_IMA_KEXEC is defined. I moved it to ima_kexec_fdt.c

Instead of ima_kexec_fdt.c, where ima_dump_measurement_list() and ima_add_kexec_buffer() are defined, perhaps I can change the file name to "ima_kexec_buffer.c". Would that be better?

Hi Lakshmi,

Quoted 36 lines · Lakshmi Ramasubramanian

Quoted 24 lines · Mimi Zohar

Quoted 14 lines · Lakshmi Ramasubramanian

The functions ima_get_kexec_buffer() and ima_free_kexec_buffer(), 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.

Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to IMA subsystem. A later patch in this series will use these functions for carrying forward the IMA measurement log for ARM64.

With the above refactoring arch/powerpc/kexec/ima.c contains only functions used when CONFIG_IMA_KEXEC is enabled. Update Makefile in arch/powerpc/kexec to include arch/powerpc/kexec/ima.c only when CONFIG_IMA_KEXEC is enabled.

Quoted 8 lines

Move ima_dump_measurement_list() and ima_add_kexec_buffer() to a new file namely ima_kexec_fdt.c in IMA. Update security/integrity/ima/Makefile to include ima_kexec_fdt.c only when CONFIG_IMA_KEXEC is enabled.

Co-developed-by Prakhar Srivastava <[email protected]>

Signed-off-by Prakhar Srivastava <[email protected]>

Signed-off-by Lakshmi Ramasubramanian <[email protected]>

Quoted 6 lines

The existing support for carrying the IMA measurement list across kexec is limited to powerpc. This patch set is adding similar support for arm64, making as much of the existing code as generic as possible. However ima_dump_measurement_list() is already generic, but for some reason this patch moves it to ima_kexec_fdt.c. ima_kexec_fdt.c should be limited to device tree specific code.

I wanted to split the functions defined under CONFIG_HAVE_IMA_KEXEC and CONFIG_IMA_KEXEC to separate files so that we can get rid of #ifdef in C file and instead conditionally compile the C files (using Makefile).

Quoted 2 lines

ima_dump_measurement_list() need to be defined only when CONFIG_IMA_KEXEC is defined. I moved it to ima_kexec_fdt.c

In this case, everything in ima_kexec.c relates to carrying or restoring the measurement list. It's a logical unit. Separating them doesn't make sense.

Quoted 4 lines

Instead of ima_kexec_fdt.c, where ima_dump_measurement_list() and ima_add_kexec_buffer() are defined, perhaps I can change the file name to "ima_kexec_buffer.c". Would that be better?

I don't understand why adding support for carrying the IMA measurement across kexec on ARM64, should require any changes in the IMA loading and restoring the measurement list code itself. Please minimize the changes.