mbox series

[v9,0/8] Carry forward IMA measurement log on kexec on ARM64

Message ID 20201113192243.1993-1-nramas@linux.microsoft.com (mailing list archive)
Headers show
Series Carry forward IMA measurement log on kexec on ARM64 | expand

Message

Lakshmi Ramasubramanian Nov. 13, 2020, 7:22 p.m. UTC
On kexec file load Integrity Measurement Architecture (IMA) subsystem
may verify the IMA signature of the kernel and initramfs, and measure
it. The command line parameters passed to the kernel in the kexec call
may also be measured by IMA. A remote attestation service can verify
a TPM quote based on the TPM event log, the IMA measurement list, and
the TPM PCR data. This can be achieved only if the IMA measurement log
is carried over from the current kernel to the next kernel across
the kexec call.

powerpc already supports carrying forward the IMA measurement log on
kexec. This patch set adds support for carrying forward the IMA
measurement log on kexec on ARM64. 

This patch set moves the platform independent code defined for powerpc
such that it can be reused for other platforms as well. A chosen node
namely "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
the address and the size of the memory reserved to carry
the IMA measurement log.

This patch set has been tested for ARM64 platform using QEMU.
I would like help from the community for testing this change on powerpc.
Thanks.

This patch set is based on
commit af5043c89a8e ("Merge tag 'acpi-5.10-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm")
in https://github.com/torvalds/linux "master" branch.

Changelog:

v9
  - Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
  - Defined a new function get_ima_kexec_buffer() in
    drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
  - Changed remove_ima_kexec_buffer() to the original function name
    remove_ima_buffer()
  - Moved remove_ima_buffer() to drivers/of/ima_kexec.c
  - Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
    to security/integrity/ima/ima_kexec.c

v8:
  - Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
    delete_fdt_mem_rsv() to drivers/of/fdt.c
  - Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
    back to security/integrity/ima/ima_kexec.c

v7:
  - Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
    this function definition to kernel.
  - Moved delete_fdt_mem_rsv() definition to kernel
  - Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
    a new file namely ima_kexec_fdt.c in IMA

v6:
  - Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device
    tree and also its corresponding memory reservation in the currently
    running kernel.
  - Moved the function remove_ima_buffer() defined for powerpc to IMA
    and renamed the function to ima_remove_kexec_buffer(). Also, moved
    delete_fdt_mem_rsv() from powerpc to IMA.

v5:
  - Merged get_addr_size_cells() and do_get_kexec_buffer() into a single
    function when moving the arch independent code from powerpc to IMA
  - Reverted the change to use FDT functions in powerpc code and added
    back the original code in get_addr_size_cells() and
    do_get_kexec_buffer() for powerpc.
  - Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
    the IMA log buffer during kexec.
  - Fixed the warning reported by kernel test bot for ARM64
    arch_ima_add_kexec_buffer() - moved this function to a new file
    namely arch/arm64/kernel/ima_kexec.c

v4:
  - Submitting the patch series on behalf of the original author
    Prakhar Srivastava <prsriva@linux.microsoft.com>
  - Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
    libfdt.h so that it can be shared by multiple platforms.

v3:
Breakup patches further into separate patches.
  - Refactoring non architecture specific code out of powerpc
  - Update powerpc related code to use fdt functions
  - Update IMA buffer read related code to use of functions
  - Add support to store the memory information of the IMA
    measurement logs to be carried forward.
  - Update the property strings to align with documented nodes
    https://github.com/devicetree-org/dt-schema/pull/46

v2:
  Break patches into separate patches.
  - Powerpc related Refactoring
  - Updating the docuemntation for chosen node
  - Updating arm64 to support IMA buffer pass

v1:
  Refactoring carrying over IMA measuremnet logs over Kexec. This patch
    moves the non-architecture specific code out of powerpc and adds to
    security/ima.(Suggested by Thiago)
  Add Documentation regarding the ima-kexec-buffer node in the chosen
    node documentation

v0:
  Add a layer of abstraction to use the memory reserved by device tree
    for ima buffer pass.
  Add support for ima buffer pass using reserved memory for arm64 kexec.
    Update the arch sepcific code path in kexec file load to store the
    ima buffer in the reserved memory. The same reserved memory is read
    on kexec or cold boot.

Lakshmi Ramasubramanian (8):
  powerpc: fix compiler warnings and errors
  powerpc: Move delete_fdt_mem_rsv() to drivers/of
  ima: Define get_ima_kexec_buffer() in drivers/of
  powerpc: Use get_ima_kexec_buffer to get ima kexec buffer
  powerpc: Move remove_ima_buffer() to drivers/of
  powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to
    ima
  arm64: Store IMA log information in kimage used for kexec
  arm64: Add IMA kexec buffer to DTB

 arch/arm64/Kconfig                     |   1 +
 arch/arm64/include/asm/ima.h           |  18 +++++
 arch/arm64/include/asm/kexec.h         |   3 +
 arch/arm64/kernel/Makefile             |   1 +
 arch/arm64/kernel/ima_kexec.c          |  34 ++++++++
 arch/arm64/kernel/machine_kexec_file.c |  18 +++++
 arch/powerpc/include/asm/ima.h         |  11 +--
 arch/powerpc/include/asm/kexec.h       |   1 -
 arch/powerpc/kexec/Makefile            |   7 +-
 arch/powerpc/kexec/file_load.c         |  32 --------
 arch/powerpc/kexec/ima.c               | 106 ++-----------------------
 drivers/of/Makefile                    |   9 +++
 drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
 drivers/of/kexec_fdt.c                 |  55 +++++++++++++
 include/linux/kexec.h                  |  24 ++++++
 security/integrity/ima/ima_kexec.c     |  51 ++++++++++++
 16 files changed, 313 insertions(+), 149 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c
 create mode 100644 drivers/of/ima_kexec.c
 create mode 100644 drivers/of/kexec_fdt.c

Comments

Rob Herring (Arm) Nov. 21, 2020, 1:57 p.m. UTC | #1
On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
> 
> powerpc already supports carrying forward the IMA measurement log on
> kexec. This patch set adds support for carrying forward the IMA
> measurement log on kexec on ARM64. 
> 
> This patch set moves the platform independent code defined for powerpc
> such that it can be reused for other platforms as well. A chosen node
> namely "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
> the address and the size of the memory reserved to carry
> the IMA measurement log.
> 
> This patch set has been tested for ARM64 platform using QEMU.
> I would like help from the community for testing this change on powerpc.
> Thanks.

As I mentioned previously, please move the kexec code to drivers/of/. Or 
at least put the infrastructure in place so we're not defining public 
IMA functions and then making them static later.
 
> This patch set is based on
> commit af5043c89a8e ("Merge tag 'acpi-5.10-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm")
> in https://github.com/torvalds/linux "master" branch.
> 
> Changelog:
> 
> v9
>   - Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
>   - Defined a new function get_ima_kexec_buffer() in
>     drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
>   - Changed remove_ima_kexec_buffer() to the original function name
>     remove_ima_buffer()
>   - Moved remove_ima_buffer() to drivers/of/ima_kexec.c
>   - Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
>     to security/integrity/ima/ima_kexec.c
> 
> v8:
>   - Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
>     delete_fdt_mem_rsv() to drivers/of/fdt.c
>   - Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
>     back to security/integrity/ima/ima_kexec.c
> 
> v7:
>   - Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
>     this function definition to kernel.
>   - Moved delete_fdt_mem_rsv() definition to kernel
>   - Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
>     a new file namely ima_kexec_fdt.c in IMA
> 
> v6:
>   - Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device
>     tree and also its corresponding memory reservation in the currently
>     running kernel.
>   - Moved the function remove_ima_buffer() defined for powerpc to IMA
>     and renamed the function to ima_remove_kexec_buffer(). Also, moved
>     delete_fdt_mem_rsv() from powerpc to IMA.
> 
> v5:
>   - Merged get_addr_size_cells() and do_get_kexec_buffer() into a single
>     function when moving the arch independent code from powerpc to IMA
>   - Reverted the change to use FDT functions in powerpc code and added
>     back the original code in get_addr_size_cells() and
>     do_get_kexec_buffer() for powerpc.
>   - Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
>     the IMA log buffer during kexec.
>   - Fixed the warning reported by kernel test bot for ARM64
>     arch_ima_add_kexec_buffer() - moved this function to a new file
>     namely arch/arm64/kernel/ima_kexec.c
> 
> v4:
>   - Submitting the patch series on behalf of the original author
>     Prakhar Srivastava <prsriva@linux.microsoft.com>
>   - Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
>     libfdt.h so that it can be shared by multiple platforms.
> 
> v3:
> Breakup patches further into separate patches.
>   - Refactoring non architecture specific code out of powerpc
>   - Update powerpc related code to use fdt functions
>   - Update IMA buffer read related code to use of functions
>   - Add support to store the memory information of the IMA
>     measurement logs to be carried forward.
>   - Update the property strings to align with documented nodes
>     https://github.com/devicetree-org/dt-schema/pull/46
> 
> v2:
>   Break patches into separate patches.
>   - Powerpc related Refactoring
>   - Updating the docuemntation for chosen node
>   - Updating arm64 to support IMA buffer pass
> 
> v1:
>   Refactoring carrying over IMA measuremnet logs over Kexec. This patch
>     moves the non-architecture specific code out of powerpc and adds to
>     security/ima.(Suggested by Thiago)
>   Add Documentation regarding the ima-kexec-buffer node in the chosen
>     node documentation
> 
> v0:
>   Add a layer of abstraction to use the memory reserved by device tree
>     for ima buffer pass.
>   Add support for ima buffer pass using reserved memory for arm64 kexec.
>     Update the arch sepcific code path in kexec file load to store the
>     ima buffer in the reserved memory. The same reserved memory is read
>     on kexec or cold boot.
> 
> Lakshmi Ramasubramanian (8):
>   powerpc: fix compiler warnings and errors
>   powerpc: Move delete_fdt_mem_rsv() to drivers/of
>   ima: Define get_ima_kexec_buffer() in drivers/of
>   powerpc: Use get_ima_kexec_buffer to get ima kexec buffer
>   powerpc: Move remove_ima_buffer() to drivers/of
>   powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to
>     ima
>   arm64: Store IMA log information in kimage used for kexec
>   arm64: Add IMA kexec buffer to DTB
> 
>  arch/arm64/Kconfig                     |   1 +
>  arch/arm64/include/asm/ima.h           |  18 +++++
>  arch/arm64/include/asm/kexec.h         |   3 +
>  arch/arm64/kernel/Makefile             |   1 +
>  arch/arm64/kernel/ima_kexec.c          |  34 ++++++++
>  arch/arm64/kernel/machine_kexec_file.c |  18 +++++
>  arch/powerpc/include/asm/ima.h         |  11 +--
>  arch/powerpc/include/asm/kexec.h       |   1 -
>  arch/powerpc/kexec/Makefile            |   7 +-
>  arch/powerpc/kexec/file_load.c         |  32 --------
>  arch/powerpc/kexec/ima.c               | 106 ++-----------------------
>  drivers/of/Makefile                    |   9 +++

>  drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
>  drivers/of/kexec_fdt.c                 |  55 +++++++++++++

Does this need to be 2 files? Just kexec.c?

>  include/linux/kexec.h                  |  24 ++++++
>  security/integrity/ima/ima_kexec.c     |  51 ++++++++++++
>  16 files changed, 313 insertions(+), 149 deletions(-)
>  create mode 100644 arch/arm64/include/asm/ima.h
>  create mode 100644 arch/arm64/kernel/ima_kexec.c
>  create mode 100644 drivers/of/ima_kexec.c
>  create mode 100644 drivers/of/kexec_fdt.c
> 
> -- 
> 2.29.2
>
Lakshmi Ramasubramanian Nov. 21, 2020, 2:38 p.m. UTC | #2
On 11/21/20 5:57 AM, Rob Herring wrote:
> On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
>> On kexec file load Integrity Measurement Architecture (IMA) subsystem
>> may verify the IMA signature of the kernel and initramfs, and measure
>> it. The command line parameters passed to the kernel in the kexec call
>> may also be measured by IMA. A remote attestation service can verify
>> a TPM quote based on the TPM event log, the IMA measurement list, and
>> the TPM PCR data. This can be achieved only if the IMA measurement log
>> is carried over from the current kernel to the next kernel across
>> the kexec call.
>>
>> powerpc already supports carrying forward the IMA measurement log on
>> kexec. This patch set adds support for carrying forward the IMA
>> measurement log on kexec on ARM64.
>>
>> This patch set moves the platform independent code defined for powerpc
>> such that it can be reused for other platforms as well. A chosen node
>> namely "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
>> the address and the size of the memory reserved to carry
>> the IMA measurement log.
>>
>> This patch set has been tested for ARM64 platform using QEMU.
>> I would like help from the community for testing this change on powerpc.
>> Thanks.
> 
> As I mentioned previously, please move the kexec code to drivers/of/. Or
> at least put the infrastructure in place so we're not defining public
> IMA functions and then making them static later.

I am not sure I am following you. Could you please clarify -
In this patch series, per your suggestion, I have moved the architecture 
independent kexec code to "drivers/of".

	=> drivers/of/ima_kexec.c
	=> drivers/of/kexec_fdt.c

Please let me know if I missed something.

>   
>> This patch set is based on
>> commit af5043c89a8e ("Merge tag 'acpi-5.10-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm")
>> in https://github.com/torvalds/linux "master" branch.
>>
>> Changelog:
>>
>> v9
>>    - Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
>>    - Defined a new function get_ima_kexec_buffer() in
>>      drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
>>    - Changed remove_ima_kexec_buffer() to the original function name
>>      remove_ima_buffer()
>>    - Moved remove_ima_buffer() to drivers/of/ima_kexec.c
>>    - Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
>>      to security/integrity/ima/ima_kexec.c
>>
>> v8:
>>    - Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
>>      delete_fdt_mem_rsv() to drivers/of/fdt.c
>>    - Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
>>      back to security/integrity/ima/ima_kexec.c
>>
>> v7:
>>    - Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
>>      this function definition to kernel.
>>    - Moved delete_fdt_mem_rsv() definition to kernel
>>    - Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
>>      a new file namely ima_kexec_fdt.c in IMA
>>
>> v6:
>>    - Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device
>>      tree and also its corresponding memory reservation in the currently
>>      running kernel.
>>    - Moved the function remove_ima_buffer() defined for powerpc to IMA
>>      and renamed the function to ima_remove_kexec_buffer(). Also, moved
>>      delete_fdt_mem_rsv() from powerpc to IMA.
>>
>> v5:
>>    - Merged get_addr_size_cells() and do_get_kexec_buffer() into a single
>>      function when moving the arch independent code from powerpc to IMA
>>    - Reverted the change to use FDT functions in powerpc code and added
>>      back the original code in get_addr_size_cells() and
>>      do_get_kexec_buffer() for powerpc.
>>    - Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
>>      the IMA log buffer during kexec.
>>    - Fixed the warning reported by kernel test bot for ARM64
>>      arch_ima_add_kexec_buffer() - moved this function to a new file
>>      namely arch/arm64/kernel/ima_kexec.c
>>
>> v4:
>>    - Submitting the patch series on behalf of the original author
>>      Prakhar Srivastava <prsriva@linux.microsoft.com>
>>    - Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
>>      libfdt.h so that it can be shared by multiple platforms.
>>
>> v3:
>> Breakup patches further into separate patches.
>>    - Refactoring non architecture specific code out of powerpc
>>    - Update powerpc related code to use fdt functions
>>    - Update IMA buffer read related code to use of functions
>>    - Add support to store the memory information of the IMA
>>      measurement logs to be carried forward.
>>    - Update the property strings to align with documented nodes
>>      https://github.com/devicetree-org/dt-schema/pull/46
>>
>> v2:
>>    Break patches into separate patches.
>>    - Powerpc related Refactoring
>>    - Updating the docuemntation for chosen node
>>    - Updating arm64 to support IMA buffer pass
>>
>> v1:
>>    Refactoring carrying over IMA measuremnet logs over Kexec. This patch
>>      moves the non-architecture specific code out of powerpc and adds to
>>      security/ima.(Suggested by Thiago)
>>    Add Documentation regarding the ima-kexec-buffer node in the chosen
>>      node documentation
>>
>> v0:
>>    Add a layer of abstraction to use the memory reserved by device tree
>>      for ima buffer pass.
>>    Add support for ima buffer pass using reserved memory for arm64 kexec.
>>      Update the arch sepcific code path in kexec file load to store the
>>      ima buffer in the reserved memory. The same reserved memory is read
>>      on kexec or cold boot.
>>
>> Lakshmi Ramasubramanian (8):
>>    powerpc: fix compiler warnings and errors
>>    powerpc: Move delete_fdt_mem_rsv() to drivers/of
>>    ima: Define get_ima_kexec_buffer() in drivers/of
>>    powerpc: Use get_ima_kexec_buffer to get ima kexec buffer
>>    powerpc: Move remove_ima_buffer() to drivers/of
>>    powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to
>>      ima
>>    arm64: Store IMA log information in kimage used for kexec
>>    arm64: Add IMA kexec buffer to DTB
>>
>>   arch/arm64/Kconfig                     |   1 +
>>   arch/arm64/include/asm/ima.h           |  18 +++++
>>   arch/arm64/include/asm/kexec.h         |   3 +
>>   arch/arm64/kernel/Makefile             |   1 +
>>   arch/arm64/kernel/ima_kexec.c          |  34 ++++++++
>>   arch/arm64/kernel/machine_kexec_file.c |  18 +++++
>>   arch/powerpc/include/asm/ima.h         |  11 +--
>>   arch/powerpc/include/asm/kexec.h       |   1 -
>>   arch/powerpc/kexec/Makefile            |   7 +-
>>   arch/powerpc/kexec/file_load.c         |  32 --------
>>   arch/powerpc/kexec/ima.c               | 106 ++-----------------------
>>   drivers/of/Makefile                    |   9 +++
> 
>>   drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
>>   drivers/of/kexec_fdt.c                 |  55 +++++++++++++
> 
> Does this need to be 2 files? Just kexec.c?

Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are 
enabled on 2 different kernel CONFIGs, keeping them in 2 files enables 
us to avoid using "#ifdef" in C files.

thanks,
  -lakshmi

> 
>>   include/linux/kexec.h                  |  24 ++++++
>>   security/integrity/ima/ima_kexec.c     |  51 ++++++++++++
>>   16 files changed, 313 insertions(+), 149 deletions(-)
>>   create mode 100644 arch/arm64/include/asm/ima.h
>>   create mode 100644 arch/arm64/kernel/ima_kexec.c
>>   create mode 100644 drivers/of/ima_kexec.c
>>   create mode 100644 drivers/of/kexec_fdt.c
>>
>> -- 
>> 2.29.2
>>
Lakshmi Ramasubramanian Nov. 21, 2020, 7:47 p.m. UTC | #3
On 11/21/20 6:38 AM, Lakshmi Ramasubramanian wrote:
> On 11/21/20 5:57 AM, Rob Herring wrote:
>> On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
>>> On kexec file load Integrity Measurement Architecture (IMA) subsystem
>>> may verify the IMA signature of the kernel and initramfs, and measure
>>> it. The command line parameters passed to the kernel in the kexec call
>>> may also be measured by IMA. A remote attestation service can verify
>>> a TPM quote based on the TPM event log, the IMA measurement list, and
>>> the TPM PCR data. This can be achieved only if the IMA measurement log
>>> is carried over from the current kernel to the next kernel across
>>> the kexec call.
>>>
>>> powerpc already supports carrying forward the IMA measurement log on
>>> kexec. This patch set adds support for carrying forward the IMA
>>> measurement log on kexec on ARM64.
>>>
>>> This patch set moves the platform independent code defined for powerpc
>>> such that it can be reused for other platforms as well. A chosen node
>>> namely "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
>>> the address and the size of the memory reserved to carry
>>> the IMA measurement log.
>>>
>>> This patch set has been tested for ARM64 platform using QEMU.
>>> I would like help from the community for testing this change on powerpc.
>>> Thanks.
>>
>> As I mentioned previously, please move the kexec code to drivers/of/. Or
>> at least put the infrastructure in place so we're not defining public
>> IMA functions and then making them static later.
> 
> I am not sure I am following you. Could you please clarify -
> In this patch series, per your suggestion, I have moved the architecture 
> independent kexec code to "drivers/of".
> 
>      => drivers/of/ima_kexec.c
>      => drivers/of/kexec_fdt.c
> 
> Please let me know if I missed something.

The following two arch independent kexec functions are required only 
when CONFIG_IMA is enabled. So I moved them under security/integrity/ima

	=> ima_get_kexec_buffer()
	=> ima_free_kexec_buffer()

But the other arch independent kexec functions have been moved to 
drivers/of.

I am not sure if you wanted ima_get_kexec_buffer() and 
ima_free_kexec_buffer() to also be moved to "drivers/of". Please clarify.

thanks,
  -lakshmi

>>> This patch set is based on
>>> commit af5043c89a8e ("Merge tag 'acpi-5.10-rc4' of 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm")
>>> in https://github.com/torvalds/linux "master" branch.
>>>
>>> Changelog:
>>>
>>> v9
>>>    - Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
>>>    - Defined a new function get_ima_kexec_buffer() in
>>>      drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
>>>    - Changed remove_ima_kexec_buffer() to the original function name
>>>      remove_ima_buffer()
>>>    - Moved remove_ima_buffer() to drivers/of/ima_kexec.c
>>>    - Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
>>>      to security/integrity/ima/ima_kexec.c
>>>
>>> v8:
>>>    - Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
>>>      delete_fdt_mem_rsv() to drivers/of/fdt.c
>>>    - Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
>>>      back to security/integrity/ima/ima_kexec.c
>>>
>>> v7:
>>>    - Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
>>>      this function definition to kernel.
>>>    - Moved delete_fdt_mem_rsv() definition to kernel
>>>    - Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
>>>      a new file namely ima_kexec_fdt.c in IMA
>>>
>>> v6:
>>>    - Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the 
>>> device
>>>      tree and also its corresponding memory reservation in the currently
>>>      running kernel.
>>>    - Moved the function remove_ima_buffer() defined for powerpc to IMA
>>>      and renamed the function to ima_remove_kexec_buffer(). Also, moved
>>>      delete_fdt_mem_rsv() from powerpc to IMA.
>>>
>>> v5:
>>>    - Merged get_addr_size_cells() and do_get_kexec_buffer() into a 
>>> single
>>>      function when moving the arch independent code from powerpc to IMA
>>>    - Reverted the change to use FDT functions in powerpc code and added
>>>      back the original code in get_addr_size_cells() and
>>>      do_get_kexec_buffer() for powerpc.
>>>    - Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
>>>      the IMA log buffer during kexec.
>>>    - Fixed the warning reported by kernel test bot for ARM64
>>>      arch_ima_add_kexec_buffer() - moved this function to a new file
>>>      namely arch/arm64/kernel/ima_kexec.c
>>>
>>> v4:
>>>    - Submitting the patch series on behalf of the original author
>>>      Prakhar Srivastava <prsriva@linux.microsoft.com>
>>>    - Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
>>>      libfdt.h so that it can be shared by multiple platforms.
>>>
>>> v3:
>>> Breakup patches further into separate patches.
>>>    - Refactoring non architecture specific code out of powerpc
>>>    - Update powerpc related code to use fdt functions
>>>    - Update IMA buffer read related code to use of functions
>>>    - Add support to store the memory information of the IMA
>>>      measurement logs to be carried forward.
>>>    - Update the property strings to align with documented nodes
>>>      https://github.com/devicetree-org/dt-schema/pull/46
>>>
>>> v2:
>>>    Break patches into separate patches.
>>>    - Powerpc related Refactoring
>>>    - Updating the docuemntation for chosen node
>>>    - Updating arm64 to support IMA buffer pass
>>>
>>> v1:
>>>    Refactoring carrying over IMA measuremnet logs over Kexec. This patch
>>>      moves the non-architecture specific code out of powerpc and adds to
>>>      security/ima.(Suggested by Thiago)
>>>    Add Documentation regarding the ima-kexec-buffer node in the chosen
>>>      node documentation
>>>
>>> v0:
>>>    Add a layer of abstraction to use the memory reserved by device tree
>>>      for ima buffer pass.
>>>    Add support for ima buffer pass using reserved memory for arm64 
>>> kexec.
>>>      Update the arch sepcific code path in kexec file load to store the
>>>      ima buffer in the reserved memory. The same reserved memory is read
>>>      on kexec or cold boot.
>>>
>>> Lakshmi Ramasubramanian (8):
>>>    powerpc: fix compiler warnings and errors
>>>    powerpc: Move delete_fdt_mem_rsv() to drivers/of
>>>    ima: Define get_ima_kexec_buffer() in drivers/of
>>>    powerpc: Use get_ima_kexec_buffer to get ima kexec buffer
>>>    powerpc: Move remove_ima_buffer() to drivers/of
>>>    powerpc: Move ima_get_kexec_buffer() and ima_free_kexec_buffer() to
>>>      ima
>>>    arm64: Store IMA log information in kimage used for kexec
>>>    arm64: Add IMA kexec buffer to DTB
>>>
>>>   arch/arm64/Kconfig                     |   1 +
>>>   arch/arm64/include/asm/ima.h           |  18 +++++
>>>   arch/arm64/include/asm/kexec.h         |   3 +
>>>   arch/arm64/kernel/Makefile             |   1 +
>>>   arch/arm64/kernel/ima_kexec.c          |  34 ++++++++
>>>   arch/arm64/kernel/machine_kexec_file.c |  18 +++++
>>>   arch/powerpc/include/asm/ima.h         |  11 +--
>>>   arch/powerpc/include/asm/kexec.h       |   1 -
>>>   arch/powerpc/kexec/Makefile            |   7 +-
>>>   arch/powerpc/kexec/file_load.c         |  32 --------
>>>   arch/powerpc/kexec/ima.c               | 106 ++-----------------------
>>>   drivers/of/Makefile                    |   9 +++
>>
>>>   drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
>>>   drivers/of/kexec_fdt.c                 |  55 +++++++++++++
>>
>> Does this need to be 2 files? Just kexec.c?
> 
> Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are 
> enabled on 2 different kernel CONFIGs, keeping them in 2 files enables 
> us to avoid using "#ifdef" in C files.
> 
> thanks,
>   -lakshmi
> 
>>
>>>   include/linux/kexec.h                  |  24 ++++++
>>>   security/integrity/ima/ima_kexec.c     |  51 ++++++++++++
>>>   16 files changed, 313 insertions(+), 149 deletions(-)
>>>   create mode 100644 arch/arm64/include/asm/ima.h
>>>   create mode 100644 arch/arm64/kernel/ima_kexec.c
>>>   create mode 100644 drivers/of/ima_kexec.c
>>>   create mode 100644 drivers/of/kexec_fdt.c
>>>
>>> -- 
>>> 2.29.2
>>>
Mimi Zohar Dec. 1, 2020, 11:34 a.m. UTC | #4
On Sat, 2020-11-21 at 06:38 -0800, Lakshmi Ramasubramanian wrote:
> On 11/21/20 5:57 AM, Rob Herring wrote:
> > On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:

> >>   arch/powerpc/include/asm/kexec.h       |   1 -
> >>   arch/powerpc/kexec/Makefile            |   7 +-
> >>   arch/powerpc/kexec/file_load.c         |  32 --------
> >>   arch/powerpc/kexec/ima.c               | 106 ++-----------------------
> >>   drivers/of/Makefile                    |   9 +++
> > 
> >>   drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
> >>   drivers/of/kexec_fdt.c                 |  55 +++++++++++++
> > 
> > Does this need to be 2 files? Just kexec.c?
> 
> Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are 
> enabled on 2 different kernel CONFIGs, keeping them in 2 files enables 
> us to avoid using "#ifdef" in C files.

Normally that is true, but just as all of the kexec related functions
are co-located in ima/ima_kexec.c, co-locating all of the kexec related
functions in drivers/of makes sense.  

thanks,

Mimi
Lakshmi Ramasubramanian Dec. 1, 2020, 7:10 p.m. UTC | #5
On 12/1/20 3:34 AM, Mimi Zohar wrote:
> On Sat, 2020-11-21 at 06:38 -0800, Lakshmi Ramasubramanian wrote:
>> On 11/21/20 5:57 AM, Rob Herring wrote:
>>> On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
> 
>>>>    arch/powerpc/include/asm/kexec.h       |   1 -
>>>>    arch/powerpc/kexec/Makefile            |   7 +-
>>>>    arch/powerpc/kexec/file_load.c         |  32 --------
>>>>    arch/powerpc/kexec/ima.c               | 106 ++-----------------------
>>>>    drivers/of/Makefile                    |   9 +++
>>>
>>>>    drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
>>>>    drivers/of/kexec_fdt.c                 |  55 +++++++++++++
>>>
>>> Does this need to be 2 files? Just kexec.c?
>>
>> Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are
>> enabled on 2 different kernel CONFIGs, keeping them in 2 files enables
>> us to avoid using "#ifdef" in C files.
> 
> Normally that is true, but just as all of the kexec related functions
> are co-located in ima/ima_kexec.c, co-locating all of the kexec related
> functions in drivers/of makes sense.
> 

Sounds good - i'll make the change.
Instead of "#ifdef" will use "IS_ENABLED" macro, and define the kexec 
related functions in drivers/of/ima_kexec.c

thanks,
  -lakshmi
Mimi Zohar Dec. 1, 2020, 7:15 p.m. UTC | #6
On Tue, 2020-12-01 at 11:10 -0800, Lakshmi Ramasubramanian wrote:
> On 12/1/20 3:34 AM, Mimi Zohar wrote:
> > On Sat, 2020-11-21 at 06:38 -0800, Lakshmi Ramasubramanian wrote:
> >> On 11/21/20 5:57 AM, Rob Herring wrote:
> >>> On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
> > 
> >>>>    arch/powerpc/include/asm/kexec.h       |   1 -
> >>>>    arch/powerpc/kexec/Makefile            |   7 +-
> >>>>    arch/powerpc/kexec/file_load.c         |  32 --------
> >>>>    arch/powerpc/kexec/ima.c               | 106 ++-----------------------
> >>>>    drivers/of/Makefile                    |   9 +++
> >>>
> >>>>    drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
> >>>>    drivers/of/kexec_fdt.c                 |  55 +++++++++++++
> >>>
> >>> Does this need to be 2 files? Just kexec.c?
> >>
> >> Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are
> >> enabled on 2 different kernel CONFIGs, keeping them in 2 files enables
> >> us to avoid using "#ifdef" in C files.
> > 
> > Normally that is true, but just as all of the kexec related functions
> > are co-located in ima/ima_kexec.c, co-locating all of the kexec related
> > functions in drivers/of makes sense.
> > 
> 
> Sounds good - i'll make the change.
> Instead of "#ifdef" will use "IS_ENABLED" macro, and define the kexec 
> related functions in drivers/of/ima_kexec.c

Perhaps name the file kexec.c, as Rob suggested.

Mimi
Lakshmi Ramasubramanian Dec. 2, 2020, 2:25 a.m. UTC | #7
On 12/1/20 11:15 AM, Mimi Zohar wrote:
> On Tue, 2020-12-01 at 11:10 -0800, Lakshmi Ramasubramanian wrote:
>> On 12/1/20 3:34 AM, Mimi Zohar wrote:
>>> On Sat, 2020-11-21 at 06:38 -0800, Lakshmi Ramasubramanian wrote:
>>>> On 11/21/20 5:57 AM, Rob Herring wrote:
>>>>> On Fri, Nov 13, 2020 at 11:22:35AM -0800, Lakshmi Ramasubramanian wrote:
>>>
>>>>>>     arch/powerpc/include/asm/kexec.h       |   1 -
>>>>>>     arch/powerpc/kexec/Makefile            |   7 +-
>>>>>>     arch/powerpc/kexec/file_load.c         |  32 --------
>>>>>>     arch/powerpc/kexec/ima.c               | 106 ++-----------------------
>>>>>>     drivers/of/Makefile                    |   9 +++
>>>>>
>>>>>>     drivers/of/ima_kexec.c                 |  91 +++++++++++++++++++++
>>>>>>     drivers/of/kexec_fdt.c                 |  55 +++++++++++++
>>>>>
>>>>> Does this need to be 2 files? Just kexec.c?
>>>>
>>>> Since the functions defined in "ima_kexec.c" and "kexec_fdt.c" are
>>>> enabled on 2 different kernel CONFIGs, keeping them in 2 files enables
>>>> us to avoid using "#ifdef" in C files.
>>>
>>> Normally that is true, but just as all of the kexec related functions
>>> are co-located in ima/ima_kexec.c, co-locating all of the kexec related
>>> functions in drivers/of makes sense.
>>>
>>
>> Sounds good - i'll make the change.
>> Instead of "#ifdef" will use "IS_ENABLED" macro, and define the kexec
>> related functions in drivers/of/ima_kexec.c
> 
> Perhaps name the file kexec.c, as Rob suggested.
> 

Will do.

thanks,
  -lakshmi