On 06/13/22 at 05:01pm, Mimi Zohar wrote:

On Mon, 2022-06-13 at 10:30 +0000, Jonathan McDowell wrote:

quoted

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 and ARM64 both achieve this using device tree with a
"linux,ima-kexec-buffer" node. x86 platforms generally don't make use of
device tree, so use the setup_data mechanism to pass the IMA buffer to
the new kernel.

(Mimi, Baoquan, I haven't included your reviewed-bys because this has
 changed the compile guards around the ima_(free|get)_kexec_buffer
 functions in order to fix the warning the kernel test robot found. I
 think this is the right thing to do and avoids us compiling them on
 platforms where they won't be used. The alternative would be to drop
 the guards in ima.h that Mimi requested for v4.)hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh

Signed-off-by: Jonathan McDowell <redacted>
---
v5:
 - Guard ima_(free|get)_kexec_buffer functions with
   CONFIG_HAVE_IMA_KEXEC (kernel test robot)
 - Use setup_data_offset in setup_boot_parameters and update rather than
   calculating in call to setup_ima_state.
v4:
 - Guard ima.h function prototypes with CONFIG_HAVE_IMA_KEXEC

quoted

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index 8d374cc552be..42a6c5721a43 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c

@@ -9,6 +9,7 @@
  *  Copyright (C) 2016  IBM Corporation
  */
 
+#include <linux/ima.h>
 #include <linux/kernel.h>
 #include <linux/kexec.h>
 #include <linux/memblock.h>

@@ -115,6 +116,7 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
 	return 0;
 }
 
+#ifdef CONFIG_HAVE_IMA_KEXEC
 /**
  * ima_get_kexec_buffer - get IMA buffer from the previous kernel
  * @addr:	On successful return, set to point to the buffer contents.

@@ -173,6 +175,7 @@ int ima_free_kexec_buffer(void)
 
 	return memblock_phys_free(addr, size);
 }
+#endif

Inside ima_{get,free}_kexec_buffer(), there's no need now to test
whether CONFIG_HAVE_IMA_KEXEC is enabled.

        if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
                return -ENOTSUPP;

Indeed. The #ifdef added by Jonathan is redundant. Not sure if the
original IS_ENABLED(CONFIG_HAVE_IMA_KEXEC) checking inside
ima_{get,free}_kexec_buffer() is intended. I ever reviewed below patch,
the IS_ENABLED(CONFIG_XX) inside static function will make the function
compiled, and will be optimized out if the CONFIG_XX is not enabled.
However, it only has effect on static function. Here,
ima_{get,free}_kexec_buffer() is not static, likely we should remove the
inside IS_ENABLED() checking.

commit 4ece09be9913a87ff99ea347fd7e7adad5bdbc8f
Author: Jisheng Zhang [off-list ref]
Date:   Wed Mar 23 16:06:39 2022 -0700

    x86/setup: use IS_ENABLED(CONFIG_KEXEC_CORE) instead of #ifdef
    
    Replace the conditional compilation using "#ifdef CONFIG_KEXEC_CORE" by a
    check for "IS_ENABLED(CONFIG_KEXEC_CORE)", to simplify the code and
    increase compile coverage.

Other than this one Mimi pointed out, this patch looks good to me, thx.

Reviewed-by: Baoquan He <bhe@redhat.com>

quoted

 
 /**
  * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt

diff --git a/include/linux/ima.h b/include/linux/ima.h