On 04/02/25 at 05:47am, steven chen wrote:
> The current kernel behavior is IMA measurements snapshot is taken at
> kexec 'load' and not at kexec 'execute'. IMA log is then carried
> over to the new kernel after kexec 'execute'. However, the time gap
> between kexec load and kexec reboot can be very long. During this
> time window, new events extended into TPM PCRs miss the chance
> to be carried over to the second kernel.
>
> To address the above, the following approach is proposed:
> - Allocate the necessary buffer during the kexec load phase.
> - Populate this buffer with the IMA measurements during
> the kexec execute phase.
>
> In the current implementation, a local variable "file" of type seq_file
> is used in the API ima_dump_measurement_list() to store the IMA measurements
> to be carried over across kexec system call. To make this buffer accessible
> at kexec 'execute' time, rename it to "ima_kexec_file" before making it
> a file variable to better reflect its purpose.
>
> Renaming the local variable "file" of type seq_file defined in the
> ima_dump_measurement_list function to "ima_kexec_file" will improve code
> readability and maintainability by making the variable's role more explicit.
Seems it's clearer with below paragraph to replace the whole log:
=====
Rename the local variable "file" of type seq_file defined in the
ima_dump_measurement_list function to "ima_kexec_file" to improve code
readability and maintainability by making the variable's role more explicit.
=====
The code change looks good to me.
>
> Suggested-by: Mimi Zohar <zo...@linux.ibm.com>
> Signed-off-by: steven chen <chen...@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
If there's code change in patch content, the reviewing tag should be
reset so that reviewing is taken again on the new change.
> ---
> security/integrity/ima/ima_kexec.c | 31 +++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/security/integrity/ima/ima_kexec.c
> b/security/integrity/ima/ima_kexec.c
> index 9d45f4d26f73..650beb74346c 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -18,30 +18,30 @@
> static int ima_dump_measurement_list(unsigned long *buffer_size, void
> **buffer,
> unsigned long segment_size)
> {
> + struct seq_file ima_kexec_file;
> struct ima_queue_entry *qe;
> - struct seq_file file;
> struct ima_kexec_hdr khdr;
> int ret = 0;
>
> /* segment size can't change between kexec load and execute */
> - file.buf = vmalloc(segment_size);
> - if (!file.buf) {
> + ima_kexec_file.buf = vmalloc(segment_size);
> + if (!ima_kexec_file.buf) {
> ret = -ENOMEM;
> goto out;
> }
>
> - file.file = NULL;
> - file.size = segment_size;
> - file.read_pos = 0;
> - file.count = sizeof(khdr); /* reserved space */
> + ima_kexec_file.file = NULL;
> + ima_kexec_file.size = segment_size;
> + ima_kexec_file.read_pos = 0;
> + ima_kexec_file.count = sizeof(khdr); /* reserved space */
>
> memset(&khdr, 0, sizeof(khdr));
> khdr.version = 1;
> /* This is an append-only list, no need to hold the RCU read lock */
> list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
> - if (file.count < file.size) {
> + if (ima_kexec_file.count < ima_kexec_file.size) {
> khdr.count++;
> - ima_measurements_show(&file, qe);
> + ima_measurements_show(&ima_kexec_file, qe);
> } else {
> ret = -EINVAL;
> break;
> @@ -55,23 +55,24 @@ static int ima_dump_measurement_list(unsigned long
> *buffer_size, void **buffer,
> * fill in reserved space with some buffer details
> * (eg. version, buffer size, number of measurements)
> */
> - khdr.buffer_size = file.count;
> + khdr.buffer_size = ima_kexec_file.count;
> if (ima_canonical_fmt) {
> khdr.version = cpu_to_le16(khdr.version);
> khdr.count = cpu_to_le64(khdr.count);
> khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
> }
> - memcpy(file.buf, &khdr, sizeof(khdr));
> + memcpy(ima_kexec_file.buf, &khdr, sizeof(khdr));
>
> print_hex_dump_debug("ima dump: ", DUMP_PREFIX_NONE, 16, 1,
> - file.buf, file.count < 100 ? file.count : 100,
> + ima_kexec_file.buf, ima_kexec_file.count < 100 ?
> + ima_kexec_file.count : 100,
> true);
>
> - *buffer_size = file.count;
> - *buffer = file.buf;
> + *buffer_size = ima_kexec_file.count;
> + *buffer = ima_kexec_file.buf;
> out:
> if (ret == -EINVAL)
> - vfree(file.buf);
> + vfree(ima_kexec_file.buf);
> return ret;
> }
>
> --
> 2.25.1
>
