* [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file"
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 2:23 ` Baoquan He
2025-04-08 4:37 ` Mimi Zohar
2025-04-02 12:47 ` [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf() steven chen
` (7 subsequent siblings)
8 siblings, 2 replies; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
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.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file"
2025-04-02 12:47 ` [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file" steven chen
@ 2025-04-08 2:23 ` Baoquan He
2025-04-08 4:37 ` Mimi Zohar
1 sibling, 0 replies; 35+ messages in thread
From: Baoquan He @ 2025-04-08 2:23 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
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 <zohar@linux.ibm.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file"
2025-04-02 12:47 ` [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file" steven chen
2025-04-08 2:23 ` Baoquan He
@ 2025-04-08 4:37 ` Mimi Zohar
1 sibling, 0 replies; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 4:37 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
Please fix the Subject line: change set_file -> seq_file
On Wed, 2025-04-02 at 05:47 -0700, 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.
Only the reason for the variable name change is needed. Please remove
everything else.
>
> 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.
As previously mentioned, there is a difference when naming local and file static
global variables. Variable naming is described
https://www.kernel.org/doc/html/v4.10/process/coding-style.html#naming
-> Before making the local ima_dump_measurement_list() seq_file "file" variable
file static global, rename it to ima_kexec_file.
thanks,
Mimi
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
2025-04-02 12:47 ` [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file" steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 2:58 ` Baoquan He
2025-04-08 4:07 ` Mimi Zohar
2025-04-02 12:47 ` [PATCH v11 3/9] kexec: define functions to map and unmap segments steven chen
` (6 subsequent siblings)
8 siblings, 2 replies; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
In the current implementation, the ima_dump_measurement_list() API is
called during the kexec "load" phase, where a buffer is allocated and
the measurement records are copied. Due to this, new events added after
kexec load but before kexec execute are not carried over to the new kernel
during kexec operation
To allow the buffer allocation and population to be separated into distinct
steps, make the function local seq_file "ima_kexec_file" to a file variable.
Carrying the IMA measurement list across kexec requires allocating a
buffer and copying the measurement records. Separate allocating the
buffer and copying the measurement records into separate functions in
order to allocate the buffer at kexec 'load' and copy the measurements
at kexec 'execute'.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 650beb74346c..b12ac3619b8f 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,26 +15,46 @@
#include "ima.h"
#ifdef CONFIG_IMA_KEXEC
+static struct seq_file ima_kexec_file;
+
+static void ima_free_kexec_file_buf(struct seq_file *sf)
+{
+ vfree(sf->buf);
+ sf->buf = NULL;
+ sf->size = 0;
+ sf->read_pos = 0;
+ sf->count = 0;
+}
+
+static int ima_alloc_kexec_file_buf(size_t segment_size)
+{
+ ima_free_kexec_file_buf(&ima_kexec_file);
+
+ /* segment size can't change between kexec load and execute */
+ ima_kexec_file.buf = vmalloc(segment_size);
+ if (!ima_kexec_file.buf)
+ return -ENOMEM;
+
+ ima_kexec_file.size = segment_size;
+ ima_kexec_file.read_pos = 0;
+ ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
+
+ return 0;
+}
+
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 ima_kexec_hdr khdr;
int ret = 0;
/* segment size can't change between kexec load and execute */
- ima_kexec_file.buf = vmalloc(segment_size);
if (!ima_kexec_file.buf) {
- ret = -ENOMEM;
- goto out;
+ pr_err("Kexec file buf not allocated\n");
+ return -EINVAL;
}
- 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 */
@@ -71,8 +91,6 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
*buffer_size = ima_kexec_file.count;
*buffer = ima_kexec_file.buf;
out:
- if (ret == -EINVAL)
- vfree(ima_kexec_file.buf);
return ret;
}
@@ -111,6 +129,12 @@ void ima_add_kexec_buffer(struct kimage *image)
return;
}
+ ret = ima_alloc_kexec_file_buf(kexec_segment_size);
+ if (ret < 0) {
+ pr_err("Not enough memory for the kexec measurement buffer.\n");
+ return;
+ }
+
ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
kexec_segment_size);
if (!kexec_buffer) {
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-02 12:47 ` [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf() steven chen
@ 2025-04-08 2:58 ` Baoquan He
2025-04-08 4:07 ` Mimi Zohar
1 sibling, 0 replies; 35+ messages in thread
From: Baoquan He @ 2025-04-08 2:58 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/02/25 at 05:47am, steven chen wrote:
> In the current implementation, the ima_dump_measurement_list() API is
> called during the kexec "load" phase, where a buffer is allocated and
> the measurement records are copied. Due to this, new events added after
> kexec load but before kexec execute are not carried over to the new kernel
> during kexec operation
>
> To allow the buffer allocation and population to be separated into distinct
> steps, make the function local seq_file "ima_kexec_file" to a file variable.
>
> Carrying the IMA measurement list across kexec requires allocating a
> buffer and copying the measurement records. Separate allocating the
> buffer and copying the measurement records into separate functions in
> order to allocate the buffer at kexec 'load' and copy the measurements
> at kexec 'execute'.
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 11 deletions(-)
LGTM,
Acked-by: Baoquan He <bhe@redhat.com>
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 650beb74346c..b12ac3619b8f 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -15,26 +15,46 @@
> #include "ima.h"
>
> #ifdef CONFIG_IMA_KEXEC
> +static struct seq_file ima_kexec_file;
> +
> +static void ima_free_kexec_file_buf(struct seq_file *sf)
> +{
> + vfree(sf->buf);
> + sf->buf = NULL;
> + sf->size = 0;
> + sf->read_pos = 0;
> + sf->count = 0;
> +}
> +
> +static int ima_alloc_kexec_file_buf(size_t segment_size)
> +{
> + ima_free_kexec_file_buf(&ima_kexec_file);
> +
> + /* segment size can't change between kexec load and execute */
> + ima_kexec_file.buf = vmalloc(segment_size);
> + if (!ima_kexec_file.buf)
> + return -ENOMEM;
> +
> + ima_kexec_file.size = segment_size;
> + ima_kexec_file.read_pos = 0;
> + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> +
> + return 0;
> +}
> +
> 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 ima_kexec_hdr khdr;
> int ret = 0;
>
> /* segment size can't change between kexec load and execute */
> - ima_kexec_file.buf = vmalloc(segment_size);
> if (!ima_kexec_file.buf) {
> - ret = -ENOMEM;
> - goto out;
> + pr_err("Kexec file buf not allocated\n");
> + return -EINVAL;
> }
>
> - 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 */
> @@ -71,8 +91,6 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
> *buffer_size = ima_kexec_file.count;
> *buffer = ima_kexec_file.buf;
> out:
> - if (ret == -EINVAL)
> - vfree(ima_kexec_file.buf);
> return ret;
> }
>
> @@ -111,6 +129,12 @@ void ima_add_kexec_buffer(struct kimage *image)
> return;
> }
>
> + ret = ima_alloc_kexec_file_buf(kexec_segment_size);
> + if (ret < 0) {
> + pr_err("Not enough memory for the kexec measurement buffer.\n");
> + return;
> + }
> +
> ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
> kexec_segment_size);
> if (!kexec_buffer) {
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-02 12:47 ` [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf() steven chen
2025-04-08 2:58 ` Baoquan He
@ 2025-04-08 4:07 ` Mimi Zohar
2025-04-08 4:39 ` Baoquan He
1 sibling, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 4:07 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> In the current implementation, the ima_dump_measurement_list() API is
> called during the kexec "load" phase, where a buffer is allocated and
> the measurement records are copied. Due to this, new events added after
> kexec load but before kexec execute are not carried over to the new kernel
> during kexec operation
Repeating this here is unnecessary.
>
> To allow the buffer allocation and population to be separated into distinct
> steps, make the function local seq_file "ima_kexec_file" to a file variable.
This change was already made in [PATCH v11 1/9] ima: rename variable the
set_file "file" to "ima_kexec_file". Please remove.
>
> Carrying the IMA measurement list across kexec requires allocating a
> buffer and copying the measurement records. Separate allocating the
> buffer and copying the measurement records into separate functions in
> order to allocate the buffer at kexec 'load' and copy the measurements
> at kexec 'execute'.
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 650beb74346c..b12ac3619b8f 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -15,26 +15,46 @@
> #include "ima.h"
>
> #ifdef CONFIG_IMA_KEXEC
> +static struct seq_file ima_kexec_file;
> +
> +static void ima_free_kexec_file_buf(struct seq_file *sf)
> +{
> + vfree(sf->buf);
> + sf->buf = NULL;
> + sf->size = 0;
> + sf->read_pos = 0;
> + sf->count = 0;
> +}
> +
> +static int ima_alloc_kexec_file_buf(size_t segment_size)
> +{
> + ima_free_kexec_file_buf(&ima_kexec_file);
After moving the vfree() here at this stage in the patch set, the IMA
measurement list fails to verify when doing two consecutive "kexec -s -l"
with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
copy from kexec load to execute" the IMA measurement list verifies properly with
the vfree() here.
> +
> + /* segment size can't change between kexec load and execute */
> + ima_kexec_file.buf = vmalloc(segment_size);
> + if (!ima_kexec_file.buf)
> + return -ENOMEM;
> +
> + ima_kexec_file.size = segment_size;
> + ima_kexec_file.read_pos = 0;
> + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-08 4:07 ` Mimi Zohar
@ 2025-04-08 4:39 ` Baoquan He
2025-04-08 5:03 ` Mimi Zohar
0 siblings, 1 reply; 35+ messages in thread
From: Baoquan He @ 2025-04-08 4:39 UTC (permalink / raw)
To: Mimi Zohar
Cc: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/08/25 at 12:07am, Mimi Zohar wrote:
> On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> > In the current implementation, the ima_dump_measurement_list() API is
> > called during the kexec "load" phase, where a buffer is allocated and
> > the measurement records are copied. Due to this, new events added after
> > kexec load but before kexec execute are not carried over to the new kernel
> > during kexec operation
>
> Repeating this here is unnecessary.
> >
> > To allow the buffer allocation and population to be separated into distinct
> > steps, make the function local seq_file "ima_kexec_file" to a file variable.
>
> This change was already made in [PATCH v11 1/9] ima: rename variable the
> set_file "file" to "ima_kexec_file". Please remove.
>
> >
> > Carrying the IMA measurement list across kexec requires allocating a
> > buffer and copying the measurement records. Separate allocating the
> > buffer and copying the measurement records into separate functions in
> > order to allocate the buffer at kexec 'load' and copy the measurements
> > at kexec 'execute'.
> >
> > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > ---
> > security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> > 1 file changed, 35 insertions(+), 11 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > index 650beb74346c..b12ac3619b8f 100644
> > --- a/security/integrity/ima/ima_kexec.c
> > +++ b/security/integrity/ima/ima_kexec.c
> > @@ -15,26 +15,46 @@
> > #include "ima.h"
> >
> > #ifdef CONFIG_IMA_KEXEC
> > +static struct seq_file ima_kexec_file;
> > +
> > +static void ima_free_kexec_file_buf(struct seq_file *sf)
> > +{
> > + vfree(sf->buf);
> > + sf->buf = NULL;
> > + sf->size = 0;
> > + sf->read_pos = 0;
> > + sf->count = 0;
> > +}
> > +
> > +static int ima_alloc_kexec_file_buf(size_t segment_size)
> > +{
> > + ima_free_kexec_file_buf(&ima_kexec_file);
>
> After moving the vfree() here at this stage in the patch set, the IMA
> measurement list fails to verify when doing two consecutive "kexec -s -l"
> with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
> copy from kexec load to execute" the IMA measurement list verifies properly with
> the vfree() here.
I also noticed this, patch 7 will remedy this. Put patch 7 just after
this patch or squash it into this patch?
[PATCH v11 7/9] ima: verify if the segment size has changed
>
> > +
> > + /* segment size can't change between kexec load and execute */
> > + ima_kexec_file.buf = vmalloc(segment_size);
> > + if (!ima_kexec_file.buf)
> > + return -ENOMEM;
> > +
> > + ima_kexec_file.size = segment_size;
> > + ima_kexec_file.read_pos = 0;
> > + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> > +
> > + return 0;
> > +}
> > +
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-08 4:39 ` Baoquan He
@ 2025-04-08 5:03 ` Mimi Zohar
2025-04-08 8:18 ` Baoquan He
0 siblings, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 5:03 UTC (permalink / raw)
To: Baoquan He
Cc: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Tue, 2025-04-08 at 12:39 +0800, Baoquan He wrote:
> On 04/08/25 at 12:07am, Mimi Zohar wrote:
> > On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> > > In the current implementation, the ima_dump_measurement_list() API is
> > > called during the kexec "load" phase, where a buffer is allocated and
> > > the measurement records are copied. Due to this, new events added after
> > > kexec load but before kexec execute are not carried over to the new kernel
> > > during kexec operation
> >
> > Repeating this here is unnecessary.
> > >
> > > To allow the buffer allocation and population to be separated into distinct
> > > steps, make the function local seq_file "ima_kexec_file" to a file variable.
> >
> > This change was already made in [PATCH v11 1/9] ima: rename variable the
> > set_file "file" to "ima_kexec_file". Please remove.
> >
> > >
> > > Carrying the IMA measurement list across kexec requires allocating a
> > > buffer and copying the measurement records. Separate allocating the
> > > buffer and copying the measurement records into separate functions in
> > > order to allocate the buffer at kexec 'load' and copy the measurements
> > > at kexec 'execute'.
> > >
> > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > ---
> > > security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> > > 1 file changed, 35 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > > index 650beb74346c..b12ac3619b8f 100644
> > > --- a/security/integrity/ima/ima_kexec.c
> > > +++ b/security/integrity/ima/ima_kexec.c
> > > @@ -15,26 +15,46 @@
> > > #include "ima.h"
> > >
> > > #ifdef CONFIG_IMA_KEXEC
> > > +static struct seq_file ima_kexec_file;
> > > +
> > > +static void ima_free_kexec_file_buf(struct seq_file *sf)
> > > +{
> > > + vfree(sf->buf);
> > > + sf->buf = NULL;
> > > + sf->size = 0;
> > > + sf->read_pos = 0;
> > > + sf->count = 0;
> > > +}
> > > +
> > > +static int ima_alloc_kexec_file_buf(size_t segment_size)
> > > +{
> > > + ima_free_kexec_file_buf(&ima_kexec_file);
> >
> > After moving the vfree() here at this stage in the patch set, the IMA
> > measurement list fails to verify when doing two consecutive "kexec -s -l"
> > with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
> > copy from kexec load to execute" the IMA measurement list verifies properly with
> > the vfree() here.
>
> I also noticed this, patch 7 will remedy this. Put patch 7 just after
> this patch or squash it into this patch?
>
> [PATCH v11 7/9] ima: verify if the segment size has changed
I'm glad you noticed this too! I've been staring at it for a while, not knowing
what to do.
"ima: verify if the segment size has changed" is new to v11. It was originally
part of this patch. My comment on v10 was:
The call to ima_reset_kexec_file() in ima_add_kexec_buffer() resets
ima_kexec_file.buf() hiding the fact that the above test always fails and falls
through. As a result, 'buf' is always being re-allocated.
and
Instead of adding and then removing the ima_reset_kexec_file() call from
ima_add_kexec_buffer(), defer adding the segment size test to when it is
actually possible for the segment size to change. Please make the segment size
test as a separate patch.
ima_reset_kexec_file() will then only be called by ima_free_kexec_file_buf().
Inline the ima_reset_kexec_file() code in ima_free_kexec_file_buf().
>
> >
> > > +
> > > + /* segment size can't change between kexec load and execute */
> > > + ima_kexec_file.buf = vmalloc(segment_size);
> > > + if (!ima_kexec_file.buf)
> > > + return -ENOMEM;
> > > +
> > > + ima_kexec_file.size = segment_size;
> > > + ima_kexec_file.read_pos = 0;
> > > + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> > > +
> > > + return 0;
> > > +}
> > > +
> >
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-08 5:03 ` Mimi Zohar
@ 2025-04-08 8:18 ` Baoquan He
2025-04-08 12:23 ` Mimi Zohar
0 siblings, 1 reply; 35+ messages in thread
From: Baoquan He @ 2025-04-08 8:18 UTC (permalink / raw)
To: Mimi Zohar
Cc: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/08/25 at 01:03am, Mimi Zohar wrote:
> On Tue, 2025-04-08 at 12:39 +0800, Baoquan He wrote:
> > On 04/08/25 at 12:07am, Mimi Zohar wrote:
> > > On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> > > > In the current implementation, the ima_dump_measurement_list() API is
> > > > called during the kexec "load" phase, where a buffer is allocated and
> > > > the measurement records are copied. Due to this, new events added after
> > > > kexec load but before kexec execute are not carried over to the new kernel
> > > > during kexec operation
> > >
> > > Repeating this here is unnecessary.
> > > >
> > > > To allow the buffer allocation and population to be separated into distinct
> > > > steps, make the function local seq_file "ima_kexec_file" to a file variable.
> > >
> > > This change was already made in [PATCH v11 1/9] ima: rename variable the
> > > set_file "file" to "ima_kexec_file". Please remove.
> > >
> > > >
> > > > Carrying the IMA measurement list across kexec requires allocating a
> > > > buffer and copying the measurement records. Separate allocating the
> > > > buffer and copying the measurement records into separate functions in
> > > > order to allocate the buffer at kexec 'load' and copy the measurements
> > > > at kexec 'execute'.
> > > >
> > > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > > ---
> > > > security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> > > > 1 file changed, 35 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > > > index 650beb74346c..b12ac3619b8f 100644
> > > > --- a/security/integrity/ima/ima_kexec.c
> > > > +++ b/security/integrity/ima/ima_kexec.c
> > > > @@ -15,26 +15,46 @@
> > > > #include "ima.h"
> > > >
> > > > #ifdef CONFIG_IMA_KEXEC
> > > > +static struct seq_file ima_kexec_file;
> > > > +
> > > > +static void ima_free_kexec_file_buf(struct seq_file *sf)
> > > > +{
> > > > + vfree(sf->buf);
> > > > + sf->buf = NULL;
> > > > + sf->size = 0;
> > > > + sf->read_pos = 0;
> > > > + sf->count = 0;
> > > > +}
> > > > +
> > > > +static int ima_alloc_kexec_file_buf(size_t segment_size)
> > > > +{
> > > > + ima_free_kexec_file_buf(&ima_kexec_file);
> > >
> > > After moving the vfree() here at this stage in the patch set, the IMA
> > > measurement list fails to verify when doing two consecutive "kexec -s -l"
> > > with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
> > > copy from kexec load to execute" the IMA measurement list verifies properly with
> > > the vfree() here.
> >
> > I also noticed this, patch 7 will remedy this. Put patch 7 just after
> > this patch or squash it into this patch?
> >
> > [PATCH v11 7/9] ima: verify if the segment size has changed
>
> I'm glad you noticed this too! I've been staring at it for a while, not knowing
> what to do.
>
> "ima: verify if the segment size has changed" is new to v11. It was originally
> part of this patch. My comment on v10 was:
>
> The call to ima_reset_kexec_file() in ima_add_kexec_buffer() resets
> ima_kexec_file.buf() hiding the fact that the above test always fails and falls
> through. As a result, 'buf' is always being re-allocated.
>
> and
>
> Instead of adding and then removing the ima_reset_kexec_file() call from
> ima_add_kexec_buffer(), defer adding the segment size test to when it is
> actually possible for the segment size to change. Please make the segment size
> test as a separate patch.
>
> ima_reset_kexec_file() will then only be called by ima_free_kexec_file_buf().
> Inline the ima_reset_kexec_file() code in ima_free_kexec_file_buf().
Thanks for deliberating on this and the details sharing, Mimi.
It could be fine if we add note in patch 2 log to mention the possible
failure. With my understanding, commit/patch bisectable means it won't
break compiling and block the testing. The failure you are concerned
about is not a blocker, right? And people won't back port partial
patches of this series.
Nore sure if there's another better way we can take or detour.
Thanks
Baoquan
>
> >
> > >
> > > > +
> > > > + /* segment size can't change between kexec load and execute */
> > > > + ima_kexec_file.buf = vmalloc(segment_size);
> > > > + if (!ima_kexec_file.buf)
> > > > + return -ENOMEM;
> > > > +
> > > > + ima_kexec_file.size = segment_size;
> > > > + ima_kexec_file.read_pos = 0;
> > > > + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> >
> >
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-08 8:18 ` Baoquan He
@ 2025-04-08 12:23 ` Mimi Zohar
2025-04-08 15:02 ` Baoquan He
0 siblings, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 12:23 UTC (permalink / raw)
To: Baoquan He
Cc: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Tue, 2025-04-08 at 16:18 +0800, Baoquan He wrote:
> On 04/08/25 at 01:03am, Mimi Zohar wrote:
> > On Tue, 2025-04-08 at 12:39 +0800, Baoquan He wrote:
> > > On 04/08/25 at 12:07am, Mimi Zohar wrote:
> > > > On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> > > > > In the current implementation, the ima_dump_measurement_list() API is
> > > > > called during the kexec "load" phase, where a buffer is allocated and
> > > > > the measurement records are copied. Due to this, new events added after
> > > > > kexec load but before kexec execute are not carried over to the new kernel
> > > > > during kexec operation
> > > >
> > > > Repeating this here is unnecessary.
> > > > >
> > > > > To allow the buffer allocation and population to be separated into distinct
> > > > > steps, make the function local seq_file "ima_kexec_file" to a file variable.
> > > >
> > > > This change was already made in [PATCH v11 1/9] ima: rename variable the
> > > > set_file "file" to "ima_kexec_file". Please remove.
> > > >
> > > > >
> > > > > Carrying the IMA measurement list across kexec requires allocating a
> > > > > buffer and copying the measurement records. Separate allocating the
> > > > > buffer and copying the measurement records into separate functions in
> > > > > order to allocate the buffer at kexec 'load' and copy the measurements
> > > > > at kexec 'execute'.
> > > > >
> > > > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > > > ---
> > > > > security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> > > > > 1 file changed, 35 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > > > > index 650beb74346c..b12ac3619b8f 100644
> > > > > --- a/security/integrity/ima/ima_kexec.c
> > > > > +++ b/security/integrity/ima/ima_kexec.c
> > > > > @@ -15,26 +15,46 @@
> > > > > #include "ima.h"
> > > > >
> > > > > #ifdef CONFIG_IMA_KEXEC
> > > > > +static struct seq_file ima_kexec_file;
> > > > > +
> > > > > +static void ima_free_kexec_file_buf(struct seq_file *sf)
> > > > > +{
> > > > > + vfree(sf->buf);
> > > > > + sf->buf = NULL;
> > > > > + sf->size = 0;
> > > > > + sf->read_pos = 0;
> > > > > + sf->count = 0;
> > > > > +}
> > > > > +
> > > > > +static int ima_alloc_kexec_file_buf(size_t segment_size)
> > > > > +{
> > > > > + ima_free_kexec_file_buf(&ima_kexec_file);
> > > >
> > > > After moving the vfree() here at this stage in the patch set, the IMA
> > > > measurement list fails to verify when doing two consecutive "kexec -s -l"
> > > > with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
> > > > copy from kexec load to execute" the IMA measurement list verifies properly with
> > > > the vfree() here.
> > >
> > > I also noticed this, patch 7 will remedy this. Put patch 7 just after
> > > this patch or squash it into this patch?
> > >
> > > [PATCH v11 7/9] ima: verify if the segment size has changed
> >
> > I'm glad you noticed this too! I've been staring at it for a while, not knowing
> > what to do.
> >
> > "ima: verify if the segment size has changed" is new to v11. It was originally
> > part of this patch. My comment on v10 was:
> >
> > The call to ima_reset_kexec_file() in ima_add_kexec_buffer() resets
> > ima_kexec_file.buf() hiding the fact that the above test always fails and falls
> > through. As a result, 'buf' is always being re-allocated.
> >
> > and
> >
> > Instead of adding and then removing the ima_reset_kexec_file() call from
> > ima_add_kexec_buffer(), defer adding the segment size test to when it is
> > actually possible for the segment size to change. Please make the segment size
> > test as a separate patch.
> >
> > ima_reset_kexec_file() will then only be called by ima_free_kexec_file_buf().
> > Inline the ima_reset_kexec_file() code in ima_free_kexec_file_buf().
>
> Thanks for deliberating on this and the details sharing, Mimi.
>
> It could be fine if we add note in patch 2 log to mention the possible
> failure. With my understanding, commit/patch bisectable means it won't
> break compiling and block the testing. The failure you are concerned
> about is not a blocker, right? And people won't back port partial
> patches of this series.
>
> Nore sure if there's another better way we can take or detour.
Right, doing two consecutive kexec loads in a row is not common and won't block
testing. Patch readability is more important, in this case, at least to me.
I'm fine with your suggestion.
Thanks, Boaquan.
> >
> > >
> > > >
> > > > > +
> > > > > + /* segment size can't change between kexec load and execute */
> > > > > + ima_kexec_file.buf = vmalloc(segment_size);
> > > > > + if (!ima_kexec_file.buf)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + ima_kexec_file.size = segment_size;
> > > > > + ima_kexec_file.read_pos = 0;
> > > > > + ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > >
> > >
> > >
> >
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf()
2025-04-08 12:23 ` Mimi Zohar
@ 2025-04-08 15:02 ` Baoquan He
0 siblings, 0 replies; 35+ messages in thread
From: Baoquan He @ 2025-04-08 15:02 UTC (permalink / raw)
To: Mimi Zohar
Cc: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/08/25 at 08:23am, Mimi Zohar wrote:
> On Tue, 2025-04-08 at 16:18 +0800, Baoquan He wrote:
> > On 04/08/25 at 01:03am, Mimi Zohar wrote:
> > > On Tue, 2025-04-08 at 12:39 +0800, Baoquan He wrote:
> > > > On 04/08/25 at 12:07am, Mimi Zohar wrote:
> > > > > On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> > > > > > In the current implementation, the ima_dump_measurement_list() API is
> > > > > > called during the kexec "load" phase, where a buffer is allocated and
> > > > > > the measurement records are copied. Due to this, new events added after
> > > > > > kexec load but before kexec execute are not carried over to the new kernel
> > > > > > during kexec operation
> > > > >
> > > > > Repeating this here is unnecessary.
> > > > > >
> > > > > > To allow the buffer allocation and population to be separated into distinct
> > > > > > steps, make the function local seq_file "ima_kexec_file" to a file variable.
> > > > >
> > > > > This change was already made in [PATCH v11 1/9] ima: rename variable the
> > > > > set_file "file" to "ima_kexec_file". Please remove.
> > > > >
> > > > > >
> > > > > > Carrying the IMA measurement list across kexec requires allocating a
> > > > > > buffer and copying the measurement records. Separate allocating the
> > > > > > buffer and copying the measurement records into separate functions in
> > > > > > order to allocate the buffer at kexec 'load' and copy the measurements
> > > > > > at kexec 'execute'.
> > > > > >
> > > > > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > > > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > > > > ---
> > > > > > security/integrity/ima/ima_kexec.c | 46 +++++++++++++++++++++++-------
> > > > > > 1 file changed, 35 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > > > > > index 650beb74346c..b12ac3619b8f 100644
> > > > > > --- a/security/integrity/ima/ima_kexec.c
> > > > > > +++ b/security/integrity/ima/ima_kexec.c
> > > > > > @@ -15,26 +15,46 @@
> > > > > > #include "ima.h"
> > > > > >
> > > > > > #ifdef CONFIG_IMA_KEXEC
> > > > > > +static struct seq_file ima_kexec_file;
> > > > > > +
> > > > > > +static void ima_free_kexec_file_buf(struct seq_file *sf)
> > > > > > +{
> > > > > > + vfree(sf->buf);
> > > > > > + sf->buf = NULL;
> > > > > > + sf->size = 0;
> > > > > > + sf->read_pos = 0;
> > > > > > + sf->count = 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int ima_alloc_kexec_file_buf(size_t segment_size)
> > > > > > +{
> > > > > > + ima_free_kexec_file_buf(&ima_kexec_file);
> > > > >
> > > > > After moving the vfree() here at this stage in the patch set, the IMA
> > > > > measurement list fails to verify when doing two consecutive "kexec -s -l"
> > > > > with/without a "kexec -s -u" in between. Only after "ima: kexec: move IMA log
> > > > > copy from kexec load to execute" the IMA measurement list verifies properly with
> > > > > the vfree() here.
> > > >
> > > > I also noticed this, patch 7 will remedy this. Put patch 7 just after
> > > > this patch or squash it into this patch?
> > > >
> > > > [PATCH v11 7/9] ima: verify if the segment size has changed
> > >
> > > I'm glad you noticed this too! I've been staring at it for a while, not knowing
> > > what to do.
> > >
> > > "ima: verify if the segment size has changed" is new to v11. It was originally
> > > part of this patch. My comment on v10 was:
> > >
> > > The call to ima_reset_kexec_file() in ima_add_kexec_buffer() resets
> > > ima_kexec_file.buf() hiding the fact that the above test always fails and falls
> > > through. As a result, 'buf' is always being re-allocated.
> > >
> > > and
> > >
> > > Instead of adding and then removing the ima_reset_kexec_file() call from
> > > ima_add_kexec_buffer(), defer adding the segment size test to when it is
> > > actually possible for the segment size to change. Please make the segment size
> > > test as a separate patch.
> > >
> > > ima_reset_kexec_file() will then only be called by ima_free_kexec_file_buf().
> > > Inline the ima_reset_kexec_file() code in ima_free_kexec_file_buf().
> >
> > Thanks for deliberating on this and the details sharing, Mimi.
> >
> > It could be fine if we add note in patch 2 log to mention the possible
> > failure. With my understanding, commit/patch bisectable means it won't
> > break compiling and block the testing. The failure you are concerned
> > about is not a blocker, right? And people won't back port partial
> > patches of this series.
> >
> > Nore sure if there's another better way we can take or detour.
>
> Right, doing two consecutive kexec loads in a row is not common and won't block
> testing. Patch readability is more important, in this case, at least to me.
> I'm fine with your suggestion.
That's great, thanks for confirming.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 3/9] kexec: define functions to map and unmap segments
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
2025-04-02 12:47 ` [PATCH v11 1/9] ima: rename variable the set_file "file" to "ima_kexec_file" steven chen
2025-04-02 12:47 ` [PATCH v11 2/9] ima: define and call ima_alloc_kexec_file_buf() steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 3:10 ` Baoquan He
2025-04-02 12:47 ` [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
` (5 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
Currently, the kernel behavior during kexec load is to fetch the IMA
measurements logs and store logs in kernel memory. When a kexec reboot is
triggered, these stored logs in the kernel memory are carried over to the
second kernel. 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. This results
in a mismatch between TPM PCR quotes and the actual IMA measurements list
after kexec reboot, leading to remote attestation failure.
To solve this problem, the new design defers reading the IMA measurements
logs into the kexec buffer to the kexec reboot phase, while still allocating
the necessary buffer at kexec load time because it is not appropriate to
allocate memory at the kexec reboot moment.
The content of memory segments carried over to the new kernel during the
kexec system call can be changed at the kexec 'execute' stage, but the size
of the memory segments cannot be changed at the kexec 'execute' stage.
To copy IMA measurement logs during the kexec operation, IMA allocates
memory at the kexec 'load' stage and map the segments to the kimage
structure. The mapped address will then be used to copy IMA measurements
during the kexec 'execute' stage.
Currently, the mechanism to map and unmap segments to the kimage structure
is not available to subsystems outside of kexec.
Implement kimage_map_segment() to enable IMA to map the measurement log
list to the kimage structure during the kexec 'load' stage. This function
takes a kimage pointer, a memory address, and a size, then gathers the
source pages within the specified address range, creates an array of page
pointers, and maps these to a contiguous virtual address range. The
function returns the start virtual address of this range if successful,
or NULL on failure.
Implement kimage_unmap_segment() for unmapping segments using vunmap().
From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
include/linux/kexec.h | 6 +++++
kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f0e9f8eda7a3..7d6b12f8b8d0 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -467,13 +467,19 @@ extern bool kexec_file_dbg_print;
#define kexec_dprintk(fmt, arg...) \
do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
+extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
+extern void kimage_unmap_segment(void *buffer);
#else /* !CONFIG_KEXEC_CORE */
struct pt_regs;
struct task_struct;
+struct kimage;
static inline void __crash_kexec(struct pt_regs *regs) { }
static inline void crash_kexec(struct pt_regs *regs) { }
static inline int kexec_should_crash(struct task_struct *p) { return 0; }
static inline int kexec_crash_loaded(void) { return 0; }
+static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
+{ return NULL; }
+static inline void kimage_unmap_segment(void *buffer) { }
#define kexec_in_progress false
#endif /* CONFIG_KEXEC_CORE */
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index c0bdc1686154..a5e378e1dc7f 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -867,6 +867,60 @@ int kimage_load_segment(struct kimage *image,
return result;
}
+void *kimage_map_segment(struct kimage *image,
+ unsigned long addr, unsigned long size)
+{
+ unsigned long src_page_addr, dest_page_addr = 0;
+ unsigned long eaddr = addr + size;
+ kimage_entry_t *ptr, entry;
+ struct page **src_pages;
+ unsigned int npages;
+ void *vaddr = NULL;
+ int i;
+
+ /*
+ * Collect the source pages and map them in a contiguous VA range.
+ */
+ npages = PFN_UP(eaddr) - PFN_DOWN(addr);
+ src_pages = kmalloc_array(npages, sizeof(*src_pages), GFP_KERNEL);
+ if (!src_pages) {
+ pr_err("Could not allocate ima pages array.\n");
+ return NULL;
+ }
+
+ i = 0;
+ for_each_kimage_entry(image, ptr, entry) {
+ if (entry & IND_DESTINATION) {
+ dest_page_addr = entry & PAGE_MASK;
+ } else if (entry & IND_SOURCE) {
+ if (dest_page_addr >= addr && dest_page_addr < eaddr) {
+ src_page_addr = entry & PAGE_MASK;
+ src_pages[i++] =
+ virt_to_page(__va(src_page_addr));
+ if (i == npages)
+ break;
+ dest_page_addr += PAGE_SIZE;
+ }
+ }
+ }
+
+ /* Sanity check. */
+ WARN_ON(i < npages);
+
+ vaddr = vmap(src_pages, npages, VM_MAP, PAGE_KERNEL);
+ kfree(src_pages);
+
+ if (!vaddr)
+ pr_err("Could not map ima buffer.\n");
+
+ return vaddr;
+}
+
+void kimage_unmap_segment(void *segment_buffer)
+{
+ vunmap(segment_buffer);
+}
+
struct kexec_load_limit {
/* Mutex protects the limit count. */
struct mutex mutex;
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 3/9] kexec: define functions to map and unmap segments
2025-04-02 12:47 ` [PATCH v11 3/9] kexec: define functions to map and unmap segments steven chen
@ 2025-04-08 3:10 ` Baoquan He
2025-04-10 14:11 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Baoquan He @ 2025-04-08 3:10 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/02/25 at 05:47am, steven chen wrote:
> Currently, the kernel behavior during kexec load is to fetch the IMA
> measurements logs and store logs in kernel memory. When a kexec reboot is
> triggered, these stored logs in the kernel memory are carried over to the
> second kernel. 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. This results
> in a mismatch between TPM PCR quotes and the actual IMA measurements list
> after kexec reboot, leading to remote attestation failure.
>
> To solve this problem, the new design defers reading the IMA measurements
> logs into the kexec buffer to the kexec reboot phase, while still allocating
> the necessary buffer at kexec load time because it is not appropriate to
> allocate memory at the kexec reboot moment.
>
> The content of memory segments carried over to the new kernel during the
> kexec system call can be changed at the kexec 'execute' stage, but the size
> of the memory segments cannot be changed at the kexec 'execute' stage.
>
> To copy IMA measurement logs during the kexec operation, IMA allocates
> memory at the kexec 'load' stage and map the segments to the kimage
> structure. The mapped address will then be used to copy IMA measurements
> during the kexec 'execute' stage.
>
> Currently, the mechanism to map and unmap segments to the kimage structure
> is not available to subsystems outside of kexec.
There's no need to describe the plan of the whole series. From my point
of view, the first few patches are prepared for later change. We can
just mention this, and briefly tell what it's doing if it's complicated.
For log of this patch, I would go with:
====
Implement kimage_map_segment() to enable IMA to map the measurement log
list to the kimage structure during the kexec 'load' stage. This function
gathers the source pages within the specified address range, and maps them
to a contiguous virtual address range.
This is a preparation for later usage.
====
Other than this, the code change looks good to me.
>
> Implement kimage_map_segment() to enable IMA to map the measurement log
> list to the kimage structure during the kexec 'load' stage. This function
> takes a kimage pointer, a memory address, and a size, then gathers the
> source pages within the specified address range, creates an array of page
> pointers, and maps these to a contiguous virtual address range. The
> function returns the start virtual address of this range if successful,
> or NULL on failure.
>
> Implement kimage_unmap_segment() for unmapping segments using vunmap().
>
> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> ---
> include/linux/kexec.h | 6 +++++
> kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f0e9f8eda7a3..7d6b12f8b8d0 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -467,13 +467,19 @@ extern bool kexec_file_dbg_print;
> #define kexec_dprintk(fmt, arg...) \
> do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
>
> +extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
> +extern void kimage_unmap_segment(void *buffer);
> #else /* !CONFIG_KEXEC_CORE */
> struct pt_regs;
> struct task_struct;
> +struct kimage;
> static inline void __crash_kexec(struct pt_regs *regs) { }
> static inline void crash_kexec(struct pt_regs *regs) { }
> static inline int kexec_should_crash(struct task_struct *p) { return 0; }
> static inline int kexec_crash_loaded(void) { return 0; }
> +static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
> +{ return NULL; }
> +static inline void kimage_unmap_segment(void *buffer) { }
> #define kexec_in_progress false
> #endif /* CONFIG_KEXEC_CORE */
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index c0bdc1686154..a5e378e1dc7f 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -867,6 +867,60 @@ int kimage_load_segment(struct kimage *image,
> return result;
> }
>
> +void *kimage_map_segment(struct kimage *image,
> + unsigned long addr, unsigned long size)
> +{
> + unsigned long src_page_addr, dest_page_addr = 0;
> + unsigned long eaddr = addr + size;
> + kimage_entry_t *ptr, entry;
> + struct page **src_pages;
> + unsigned int npages;
> + void *vaddr = NULL;
> + int i;
> +
> + /*
> + * Collect the source pages and map them in a contiguous VA range.
> + */
> + npages = PFN_UP(eaddr) - PFN_DOWN(addr);
> + src_pages = kmalloc_array(npages, sizeof(*src_pages), GFP_KERNEL);
> + if (!src_pages) {
> + pr_err("Could not allocate ima pages array.\n");
> + return NULL;
> + }
> +
> + i = 0;
> + for_each_kimage_entry(image, ptr, entry) {
> + if (entry & IND_DESTINATION) {
> + dest_page_addr = entry & PAGE_MASK;
> + } else if (entry & IND_SOURCE) {
> + if (dest_page_addr >= addr && dest_page_addr < eaddr) {
> + src_page_addr = entry & PAGE_MASK;
> + src_pages[i++] =
> + virt_to_page(__va(src_page_addr));
> + if (i == npages)
> + break;
> + dest_page_addr += PAGE_SIZE;
> + }
> + }
> + }
> +
> + /* Sanity check. */
> + WARN_ON(i < npages);
> +
> + vaddr = vmap(src_pages, npages, VM_MAP, PAGE_KERNEL);
> + kfree(src_pages);
> +
> + if (!vaddr)
> + pr_err("Could not map ima buffer.\n");
> +
> + return vaddr;
> +}
> +
> +void kimage_unmap_segment(void *segment_buffer)
> +{
> + vunmap(segment_buffer);
> +}
> +
> struct kexec_load_limit {
> /* Mutex protects the limit count. */
> struct mutex mutex;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 3/9] kexec: define functions to map and unmap segments
2025-04-08 3:10 ` Baoquan He
@ 2025-04-10 14:11 ` steven chen
0 siblings, 0 replies; 35+ messages in thread
From: steven chen @ 2025-04-10 14:11 UTC (permalink / raw)
To: Baoquan He
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 4/7/2025 8:10 PM, Baoquan He wrote:
> On 04/02/25 at 05:47am, steven chen wrote:
>> Currently, the kernel behavior during kexec load is to fetch the IMA
>> measurements logs and store logs in kernel memory. When a kexec reboot is
>> triggered, these stored logs in the kernel memory are carried over to the
>> second kernel. 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. This results
>> in a mismatch between TPM PCR quotes and the actual IMA measurements list
>> after kexec reboot, leading to remote attestation failure.
>>
>> To solve this problem, the new design defers reading the IMA measurements
>> logs into the kexec buffer to the kexec reboot phase, while still allocating
>> the necessary buffer at kexec load time because it is not appropriate to
>> allocate memory at the kexec reboot moment.
>>
>> The content of memory segments carried over to the new kernel during the
>> kexec system call can be changed at the kexec 'execute' stage, but the size
>> of the memory segments cannot be changed at the kexec 'execute' stage.
>>
>> To copy IMA measurement logs during the kexec operation, IMA allocates
>> memory at the kexec 'load' stage and map the segments to the kimage
>> structure. The mapped address will then be used to copy IMA measurements
>> during the kexec 'execute' stage.
>>
>> Currently, the mechanism to map and unmap segments to the kimage structure
>> is not available to subsystems outside of kexec.
> There's no need to describe the plan of the whole series. From my point
> of view, the first few patches are prepared for later change. We can
> just mention this, and briefly tell what it's doing if it's complicated.
> For log of this patch, I would go with:
>
> ====
> Implement kimage_map_segment() to enable IMA to map the measurement log
> list to the kimage structure during the kexec 'load' stage. This function
> gathers the source pages within the specified address range, and maps them
> to a contiguous virtual address range.
>
> This is a preparation for later usage.
> ====
>
> Other than this, the code change looks good to me.
Hi Baoquan,
Thanks for your comments. Will update in next version.
Steven
>> Implement kimage_map_segment() to enable IMA to map the measurement log
>> list to the kimage structure during the kexec 'load' stage. This function
>> takes a kimage pointer, a memory address, and a size, then gathers the
>> source pages within the specified address range, creates an array of page
>> pointers, and maps these to a contiguous virtual address range. The
>> function returns the start virtual address of this range if successful,
>> or NULL on failure.
>>
>> Implement kimage_unmap_segment() for unmapping segments using vunmap().
>>
>> From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Acked-by: Baoquan He <bhe@redhat.com>
>> ---
>> include/linux/kexec.h | 6 +++++
>> kernel/kexec_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index f0e9f8eda7a3..7d6b12f8b8d0 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -467,13 +467,19 @@ extern bool kexec_file_dbg_print;
>> #define kexec_dprintk(fmt, arg...) \
>> do { if (kexec_file_dbg_print) pr_info(fmt, ##arg); } while (0)
>>
>> +extern void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size);
>> +extern void kimage_unmap_segment(void *buffer);
>> #else /* !CONFIG_KEXEC_CORE */
>> struct pt_regs;
>> struct task_struct;
>> +struct kimage;
>> static inline void __crash_kexec(struct pt_regs *regs) { }
>> static inline void crash_kexec(struct pt_regs *regs) { }
>> static inline int kexec_should_crash(struct task_struct *p) { return 0; }
>> static inline int kexec_crash_loaded(void) { return 0; }
>> +static inline void *kimage_map_segment(struct kimage *image, unsigned long addr, unsigned long size)
>> +{ return NULL; }
>> +static inline void kimage_unmap_segment(void *buffer) { }
>> #define kexec_in_progress false
>> #endif /* CONFIG_KEXEC_CORE */
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index c0bdc1686154..a5e378e1dc7f 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -867,6 +867,60 @@ int kimage_load_segment(struct kimage *image,
>> return result;
>> }
>>
>> +void *kimage_map_segment(struct kimage *image,
>> + unsigned long addr, unsigned long size)
>> +{
>> + unsigned long src_page_addr, dest_page_addr = 0;
>> + unsigned long eaddr = addr + size;
>> + kimage_entry_t *ptr, entry;
>> + struct page **src_pages;
>> + unsigned int npages;
>> + void *vaddr = NULL;
>> + int i;
>> +
>> + /*
>> + * Collect the source pages and map them in a contiguous VA range.
>> + */
>> + npages = PFN_UP(eaddr) - PFN_DOWN(addr);
>> + src_pages = kmalloc_array(npages, sizeof(*src_pages), GFP_KERNEL);
>> + if (!src_pages) {
>> + pr_err("Could not allocate ima pages array.\n");
>> + return NULL;
>> + }
>> +
>> + i = 0;
>> + for_each_kimage_entry(image, ptr, entry) {
>> + if (entry & IND_DESTINATION) {
>> + dest_page_addr = entry & PAGE_MASK;
>> + } else if (entry & IND_SOURCE) {
>> + if (dest_page_addr >= addr && dest_page_addr < eaddr) {
>> + src_page_addr = entry & PAGE_MASK;
>> + src_pages[i++] =
>> + virt_to_page(__va(src_page_addr));
>> + if (i == npages)
>> + break;
>> + dest_page_addr += PAGE_SIZE;
>> + }
>> + }
>> + }
>> +
>> + /* Sanity check. */
>> + WARN_ON(i < npages);
>> +
>> + vaddr = vmap(src_pages, npages, VM_MAP, PAGE_KERNEL);
>> + kfree(src_pages);
>> +
>> + if (!vaddr)
>> + pr_err("Could not map ima buffer.\n");
>> +
>> + return vaddr;
>> +}
>> +
>> +void kimage_unmap_segment(void *segment_buffer)
>> +{
>> + vunmap(segment_buffer);
>> +}
>> +
>> struct kexec_load_limit {
>> /* Mutex protects the limit count. */
>> struct mutex mutex;
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (2 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 3/9] kexec: define functions to map and unmap segments steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 3:17 ` Baoquan He
2025-04-02 12:47 ` [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot steven chen
` (4 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The kexec_calculate_store_digests() function calculates and stores the
digest of the segment during the kexec_file_load syscall, where the
IMA segment is also allocated.
With this series, the IMA segment will be updated with the measurement
log at the kexec execute stage when a soft reboot is initiated.
Therefore, the digests should be updated for the IMA segment in the
normal case.
The content of memory segments carried over to the new kernel during the
kexec systemcall can be changed at kexec 'execute' stage, but the size
and the location of the memory segments cannot be changed at kexec
'execute' stage.
However, during the kexec execute stage, if kexec_calculate_store_digests()
API is called to update the digest, it does not reuse the same memory
segment allocated during the kexec 'load' stage and the new memory segment
required cannot be transferred/mapped to the new kernel.
As a result, digest verification will fail in verify_sha256_digest()
after a kexec soft reboot into the new kernel. Therefore, the digest
calculation/verification of the IMA segment needs to be skipped.
To address this, skip the calculation and storage of the digest for the
IMA segment in kexec_calculate_store_digests() so that it is not added
to the purgatory_sha_regions.
Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
no change is needed in verify_sha256_digest() in this context.
With this change, the IMA segment is not included in the digest
calculation, storage, and verification.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Acked-by: Baoquan He <bhe@redhat.com>
---
include/linux/kexec.h | 3 +++
kernel/kexec_file.c | 22 ++++++++++++++++++++++
security/integrity/ima/ima_kexec.c | 3 +++
3 files changed, 28 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 7d6b12f8b8d0..107e726f2ef3 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -362,6 +362,9 @@ struct kimage {
phys_addr_t ima_buffer_addr;
size_t ima_buffer_size;
+
+ unsigned long ima_segment_index;
+ bool is_ima_segment_index_set;
#endif
/* Core ELF header buffer */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 3eedb8c226ad..606132253c79 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
}
#endif
+#ifdef CONFIG_IMA_KEXEC
+static bool check_ima_segment_index(struct kimage *image, int i)
+{
+ if (image->is_ima_segment_index_set && i == image->ima_segment_index)
+ return true;
+ else
+ return false;
+}
+#else
+static bool check_ima_segment_index(struct kimage *image, int i)
+{
+ return false;
+}
+#endif
+
static int kexec_calculate_store_digests(struct kimage *image);
/* Maximum size in bytes for kernel/initrd files. */
@@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
if (ksegment->kbuf == pi->purgatory_buf)
continue;
+ /*
+ * Skip the segment if ima_segment_index is set and matches
+ * the current index
+ */
+ if (check_ima_segment_index(image, i))
+ continue;
+
ret = crypto_shash_update(desc, ksegment->kbuf,
ksegment->bufsz);
if (ret)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index b12ac3619b8f..7e0a19c3483f 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image)
kbuf.buffer = kexec_buffer;
kbuf.bufsz = kexec_buffer_size;
kbuf.memsz = kexec_segment_size;
+ image->is_ima_segment_index_set = false;
ret = kexec_add_buffer(&kbuf);
if (ret) {
pr_err("Error passing over kexec measurement buffer.\n");
@@ -155,6 +156,8 @@ void ima_add_kexec_buffer(struct kimage *image)
image->ima_buffer_addr = kbuf.mem;
image->ima_buffer_size = kexec_segment_size;
image->ima_buffer = kexec_buffer;
+ image->ima_segment_index = image->nr_segments - 1;
+ image->is_ima_segment_index_set = true;
kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
kbuf.mem);
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot
2025-04-02 12:47 ` [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
@ 2025-04-08 3:17 ` Baoquan He
2025-04-10 14:12 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Baoquan He @ 2025-04-08 3:17 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/02/25 at 05:47am, steven chen wrote:
> The kexec_calculate_store_digests() function calculates and stores the
> digest of the segment during the kexec_file_load syscall, where the
> IMA segment is also allocated.
>
> With this series, the IMA segment will be updated with the measurement
> log at the kexec execute stage when a soft reboot is initiated.
> Therefore, the digests should be updated for the IMA segment in the
> normal case.
>
> The content of memory segments carried over to the new kernel during the
> kexec systemcall can be changed at kexec 'execute' stage, but the size
> and the location of the memory segments cannot be changed at kexec
> 'execute' stage.
>
> However, during the kexec execute stage, if kexec_calculate_store_digests()
> API is called to update the digest, it does not reuse the same memory
> segment allocated during the kexec 'load' stage and the new memory segment
> required cannot be transferred/mapped to the new kernel.
>
> As a result, digest verification will fail in verify_sha256_digest()
> after a kexec soft reboot into the new kernel. Therefore, the digest
> calculation/verification of the IMA segment needs to be skipped.
>
> To address this, skip the calculation and storage of the digest for the
> IMA segment in kexec_calculate_store_digests() so that it is not added
> to the purgatory_sha_regions.
>
> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
> no change is needed in verify_sha256_digest() in this context.
>
> With this change, the IMA segment is not included in the digest
> calculation, storage, and verification.
I would write the patch log like:
======
Currently, the function kexec_calculate_store_digests() calculates and
stores the digest of the segment during the kexec_file_load syscall,
where the IMA segment is also allocated.
Later, the IMA segment will be updated with the measurement log at the
kexec execute stage when a kexec reboot is initiated. Therefore, the
digests should be updated for the IMA segment in the normal case. The
problem is that the content of memory segments carried over to the new
kernel during the kexec systemcall can be changed at kexec 'execute'
stage, but the size and the location of the memory segments cannot be
changed at kexec 'execute' stage.
To address this, skip the calculation and storage of the digest for the
IMA segment in kexec_calculate_store_digests() so that it is not added
to the purgatory_sha_regions.
With this change, the IMA segment is not included in the digest
calculation, storage, and verification.
======
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> ---
> include/linux/kexec.h | 3 +++
> kernel/kexec_file.c | 22 ++++++++++++++++++++++
> security/integrity/ima/ima_kexec.c | 3 +++
> 3 files changed, 28 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 7d6b12f8b8d0..107e726f2ef3 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -362,6 +362,9 @@ struct kimage {
>
> phys_addr_t ima_buffer_addr;
> size_t ima_buffer_size;
> +
> + unsigned long ima_segment_index;
> + bool is_ima_segment_index_set;
> #endif
>
> /* Core ELF header buffer */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3eedb8c226ad..606132253c79 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
> }
> #endif
>
> +#ifdef CONFIG_IMA_KEXEC
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> + if (image->is_ima_segment_index_set && i == image->ima_segment_index)
> + return true;
> + else
> + return false;
> +}
> +#else
> +static bool check_ima_segment_index(struct kimage *image, int i)
> +{
> + return false;
> +}
> +#endif
> +
> static int kexec_calculate_store_digests(struct kimage *image);
>
> /* Maximum size in bytes for kernel/initrd files. */
> @@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
> if (ksegment->kbuf == pi->purgatory_buf)
> continue;
>
> + /*
> + * Skip the segment if ima_segment_index is set and matches
> + * the current index
> + */
> + if (check_ima_segment_index(image, i))
> + continue;
> +
> ret = crypto_shash_update(desc, ksegment->kbuf,
> ksegment->bufsz);
> if (ret)
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index b12ac3619b8f..7e0a19c3483f 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image)
> kbuf.buffer = kexec_buffer;
> kbuf.bufsz = kexec_buffer_size;
> kbuf.memsz = kexec_segment_size;
> + image->is_ima_segment_index_set = false;
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> pr_err("Error passing over kexec measurement buffer.\n");
> @@ -155,6 +156,8 @@ void ima_add_kexec_buffer(struct kimage *image)
> image->ima_buffer_addr = kbuf.mem;
> image->ima_buffer_size = kexec_segment_size;
> image->ima_buffer = kexec_buffer;
> + image->ima_segment_index = image->nr_segments - 1;
> + image->is_ima_segment_index_set = true;
>
> kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
> kbuf.mem);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot
2025-04-08 3:17 ` Baoquan He
@ 2025-04-10 14:12 ` steven chen
0 siblings, 0 replies; 35+ messages in thread
From: steven chen @ 2025-04-10 14:12 UTC (permalink / raw)
To: Baoquan He
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 4/7/2025 8:17 PM, Baoquan He wrote:
> On 04/02/25 at 05:47am, steven chen wrote:
>> The kexec_calculate_store_digests() function calculates and stores the
>> digest of the segment during the kexec_file_load syscall, where the
>> IMA segment is also allocated.
>>
>> With this series, the IMA segment will be updated with the measurement
>> log at the kexec execute stage when a soft reboot is initiated.
>> Therefore, the digests should be updated for the IMA segment in the
>> normal case.
>>
>> The content of memory segments carried over to the new kernel during the
>> kexec systemcall can be changed at kexec 'execute' stage, but the size
>> and the location of the memory segments cannot be changed at kexec
>> 'execute' stage.
>>
>> However, during the kexec execute stage, if kexec_calculate_store_digests()
>> API is called to update the digest, it does not reuse the same memory
>> segment allocated during the kexec 'load' stage and the new memory segment
>> required cannot be transferred/mapped to the new kernel.
>>
>> As a result, digest verification will fail in verify_sha256_digest()
>> after a kexec soft reboot into the new kernel. Therefore, the digest
>> calculation/verification of the IMA segment needs to be skipped.
>>
>> To address this, skip the calculation and storage of the digest for the
>> IMA segment in kexec_calculate_store_digests() so that it is not added
>> to the purgatory_sha_regions.
>>
>> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
>> no change is needed in verify_sha256_digest() in this context.
>>
>> With this change, the IMA segment is not included in the digest
>> calculation, storage, and verification.
> I would write the patch log like:
>
> ======
> Currently, the function kexec_calculate_store_digests() calculates and
> stores the digest of the segment during the kexec_file_load syscall,
> where the IMA segment is also allocated.
>
> Later, the IMA segment will be updated with the measurement log at the
> kexec execute stage when a kexec reboot is initiated. Therefore, the
> digests should be updated for the IMA segment in the normal case. The
> problem is that the content of memory segments carried over to the new
> kernel during the kexec systemcall can be changed at kexec 'execute'
> stage, but the size and the location of the memory segments cannot be
> changed at kexec 'execute' stage.
>
> To address this, skip the calculation and storage of the digest for the
> IMA segment in kexec_calculate_store_digests() so that it is not added
> to the purgatory_sha_regions.
>
> With this change, the IMA segment is not included in the digest
> calculation, storage, and verification.
> ======
Hi Baoquan,
I will update it in next version.
Thanks for your comments.
Steven
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>> Acked-by: Baoquan He <bhe@redhat.com>
>> ---
>> include/linux/kexec.h | 3 +++
>> kernel/kexec_file.c | 22 ++++++++++++++++++++++
>> security/integrity/ima/ima_kexec.c | 3 +++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 7d6b12f8b8d0..107e726f2ef3 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -362,6 +362,9 @@ struct kimage {
>>
>> phys_addr_t ima_buffer_addr;
>> size_t ima_buffer_size;
>> +
>> + unsigned long ima_segment_index;
>> + bool is_ima_segment_index_set;
>> #endif
>>
>> /* Core ELF header buffer */
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 3eedb8c226ad..606132253c79 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
>> }
>> #endif
>>
>> +#ifdef CONFIG_IMA_KEXEC
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> + if (image->is_ima_segment_index_set && i == image->ima_segment_index)
>> + return true;
>> + else
>> + return false;
>> +}
>> +#else
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> + return false;
>> +}
>> +#endif
>> +
>> static int kexec_calculate_store_digests(struct kimage *image);
>>
>> /* Maximum size in bytes for kernel/initrd files. */
>> @@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage *image)
>> if (ksegment->kbuf == pi->purgatory_buf)
>> continue;
>>
>> + /*
>> + * Skip the segment if ima_segment_index is set and matches
>> + * the current index
>> + */
>> + if (check_ima_segment_index(image, i))
>> + continue;
>> +
>> ret = crypto_shash_update(desc, ksegment->kbuf,
>> ksegment->bufsz);
>> if (ret)
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index b12ac3619b8f..7e0a19c3483f 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -145,6 +145,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>> kbuf.buffer = kexec_buffer;
>> kbuf.bufsz = kexec_buffer_size;
>> kbuf.memsz = kexec_segment_size;
>> + image->is_ima_segment_index_set = false;
>> ret = kexec_add_buffer(&kbuf);
>> if (ret) {
>> pr_err("Error passing over kexec measurement buffer.\n");
>> @@ -155,6 +156,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>> image->ima_buffer_addr = kbuf.mem;
>> image->ima_buffer_size = kexec_segment_size;
>> image->ima_buffer = kexec_buffer;
>> + image->ima_segment_index = image->nr_segments - 1;
>> + image->is_ima_segment_index_set = true;
>>
>> kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
>> kbuf.mem);
>> --
>> 2.25.1
>>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (3 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 4/9] ima: kexec: skip IMA segment validation after kexec soft reboot steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 14:21 ` Mimi Zohar
2025-04-02 12:47 ` [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute steven chen
` (3 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The IMA log is currently copied to the new kernel during kexec 'load'
using ima_dump_measurement_list(). However, the log copied at kexec
'load' may result in loss of IMA measurements that only occurred after
kexec "load'. Therefore, the log needs to be copied during kexec
'execute'. Setup the needed infrastructure to move the IMA log copy from
kexec 'load' to 'execute'.
Define a new IMA hook ima_update_kexec_buffer() as a stub function.
It will be used to call ima_dump_measurement_list() during kexec 'execute'.
Implement ima_kexec_post_load() function to be invoked after the new
Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
IMA buffer to a segment in the newly loaded Kernel. It also registers
the reboot notifier_block to trigger ima_update_kexec_buffer() at
kexec 'execute'.
Set the priority of register_reboot_notifier to INT_MIN to ensure that the
IMA log copy operation will happen at the end of the operation chain, which
is crucial for maintaining the integrity of the logs
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
include/linux/ima.h | 3 ++
security/integrity/ima/ima_kexec.c | 47 ++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0bae61a15b60..8e29cb4e6a01 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -32,6 +32,9 @@ static inline void ima_appraise_parse_cmdline(void) {}
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
+extern void ima_kexec_post_load(struct kimage *image);
+#else
+static inline void ima_kexec_post_load(struct kimage *image) {}
#endif
#else
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 7e0a19c3483f..e79f6caf895b 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -12,10 +12,14 @@
#include <linux/kexec.h>
#include <linux/of.h>
#include <linux/ima.h>
+#include <linux/reboot.h>
+#include <asm/page.h>
#include "ima.h"
#ifdef CONFIG_IMA_KEXEC
+static bool ima_kexec_update_registered;
static struct seq_file ima_kexec_file;
+static void *ima_kexec_buffer;
static void ima_free_kexec_file_buf(struct seq_file *sf)
{
@@ -162,6 +166,49 @@ void ima_add_kexec_buffer(struct kimage *image)
kexec_dprintk("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
kbuf.mem);
}
+
+/*
+ * Called during kexec execute so that IMA can update the measurement list.
+ */
+static int ima_update_kexec_buffer(struct notifier_block *self,
+ unsigned long action, void *data)
+{
+ return NOTIFY_OK;
+}
+
+static struct notifier_block update_buffer_nb = {
+ .notifier_call = ima_update_kexec_buffer,
+ .priority = INT_MIN
+};
+
+/*
+ * Create a mapping for the source pages that contain the IMA buffer
+ * so we can update it later.
+ */
+void ima_kexec_post_load(struct kimage *image)
+{
+ if (ima_kexec_buffer) {
+ kimage_unmap_segment(ima_kexec_buffer);
+ ima_kexec_buffer = NULL;
+ }
+
+ if (!image->ima_buffer_addr)
+ return;
+
+ ima_kexec_buffer = kimage_map_segment(image,
+ image->ima_buffer_addr,
+ image->ima_buffer_size);
+ if (!ima_kexec_buffer) {
+ pr_err("Could not map measurements buffer.\n");
+ return;
+ }
+
+ if (!ima_kexec_update_registered) {
+ register_reboot_notifier(&update_buffer_nb);
+ ima_kexec_update_registered = true;
+ }
+}
+
#endif /* IMA_KEXEC */
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot
2025-04-02 12:47 ` [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot steven chen
@ 2025-04-08 14:21 ` Mimi Zohar
2025-04-10 14:13 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 14:21 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> The IMA log is currently copied to the new kernel during kexec 'load'
> using ima_dump_measurement_list(). However, the log copied at kexec
> 'load' may result in loss of IMA measurements that only occurred after
> kexec "load'.
Ok
> Therefore, the log needs to be copied during kexec
> 'execute'.
The above line is unnecessary.
> Setup the needed infrastructure to move the IMA log copy from
> kexec 'load' to 'execute'.
>
> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
> It will be used to call ima_dump_measurement_list() during kexec 'execute'.
>
> Implement ima_kexec_post_load() function to be invoked after the new
> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
> IMA buffer to a segment in the newly loaded Kernel. It also registers
> the reboot notifier_block to trigger ima_update_kexec_buffer() at
> kexec 'execute'.
>
> Set the priority of register_reboot_notifier to INT_MIN to ensure that the
> IMA log copy operation will happen at the end of the operation chain, which
> is crucial for maintaining the integrity of the logs
Instead of ", which is crucial for maintaining the integrity of the logs"
say something like ", so that all the IMA measurement records extended into the
TPM are copied."
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Thanks, Steven. With the change to use INT_MIN, the "kexec_execute" record is
now added to the IMA measurement list, extended into the PCR, and included in
the IMA measurement list records copied.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot
2025-04-08 14:21 ` Mimi Zohar
@ 2025-04-10 14:13 ` steven chen
0 siblings, 0 replies; 35+ messages in thread
From: steven chen @ 2025-04-10 14:13 UTC (permalink / raw)
To: Mimi Zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On 4/8/2025 7:21 AM, Mimi Zohar wrote:
> On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
>> The IMA log is currently copied to the new kernel during kexec 'load'
>> using ima_dump_measurement_list(). However, the log copied at kexec
>> 'load' may result in loss of IMA measurements that only occurred after
>> kexec "load'.
> Ok
>
>> Therefore, the log needs to be copied during kexec
>> 'execute'.
> The above line is unnecessary.
>
>> Setup the needed infrastructure to move the IMA log copy from
>> kexec 'load' to 'execute'.
>>
>> Define a new IMA hook ima_update_kexec_buffer() as a stub function.
>> It will be used to call ima_dump_measurement_list() during kexec 'execute'.
>>
>> Implement ima_kexec_post_load() function to be invoked after the new
>> Kernel image has been loaded for kexec. ima_kexec_post_load() maps the
>> IMA buffer to a segment in the newly loaded Kernel. It also registers
>> the reboot notifier_block to trigger ima_update_kexec_buffer() at
>> kexec 'execute'.
>>
>> Set the priority of register_reboot_notifier to INT_MIN to ensure that the
>> IMA log copy operation will happen at the end of the operation chain, which
>> is crucial for maintaining the integrity of the logs
> Instead of ", which is crucial for maintaining the integrity of the logs"
> say something like ", so that all the IMA measurement records extended into the
> TPM are copied."
Hi Mimi,
I will update in next version.
Thanks,
Steven
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Thanks, Steven. With the change to use INT_MIN, the "kexec_execute" record is
> now added to the IMA measurement list, extended into the PCR, and included in
> the IMA measurement list records copied.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (4 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 5/9] ima: kexec: define functions to copy IMA log at soft boot steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 16:17 ` Mimi Zohar
2025-04-02 12:47 ` [PATCH v11 7/9] ima: verify if the segment size has changed steven chen
` (2 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
ima_dump_measurement_list() is called during kexec 'load', which may
result in loss of IMA measurements during kexec soft reboot. Due to
missed measurements that only occurred after kexec 'load', this function
needs to be called during kexec 'execute'.
Make the kexec_segment_size variable a local static variable within the
file, so it can be accessed during both kexec 'load' and 'execute'.
Implement the kexec_post_load() function to be invoked after the new kernel
image has been loaded for kexec. Instead of calling machine_kexec_post_load()
directly from the kexec_file_load() syscall, call kexec_post_load(), which in
turn calls machine_kexec_post_load() to maintain the original image processing.
Invoke ima_kexec_post_load() within the kexec_post_load() API only for kexec
soft reboot scenarios, excluding KEXEC_FILE_ON_CRASH.
Register a reboot notifier for the ima_update_kexec_buffer() API within
ima_kexec_post_load() to ensure it is called upon receiving a reboot
notification.
Move the ima_dump_measurement_list() call from ima_add_kexec_buffer() to
ima_update_kexec_buffer() to copy the IMA log at the kexec 'execute' stage.
When there is insufficient memory to copy all the measurement logs, copy as
much of the measurement list as possible.
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Dave Young <dyoung@redhat.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
kernel/kexec_file.c | 11 +++++++-
security/integrity/ima/ima_kexec.c | 43 ++++++++++++++++++++----------
2 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 606132253c79..b3eb515ca051 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -201,6 +201,15 @@ kimage_validate_signature(struct kimage *image)
}
#endif
+static int kexec_post_load(struct kimage *image, unsigned long flags)
+{
+#ifdef CONFIG_IMA_KEXEC
+ if (!(flags & KEXEC_FILE_ON_CRASH))
+ ima_kexec_post_load(image);
+#endif
+ return machine_kexec_post_load(image);
+}
+
/*
* In file mode list of segments is prepared by kernel. Copy relevant
* data from user space, do error checking, prepare segment list
@@ -428,7 +437,7 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
kimage_terminate(image);
- ret = machine_kexec_post_load(image);
+ ret = kexec_post_load(image, flags);
if (ret)
goto out;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index e79f6caf895b..5c3b3e0b2186 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -19,6 +19,7 @@
#ifdef CONFIG_IMA_KEXEC
static bool ima_kexec_update_registered;
static struct seq_file ima_kexec_file;
+static size_t kexec_segment_size;
static void *ima_kexec_buffer;
static void ima_free_kexec_file_buf(struct seq_file *sf)
@@ -72,9 +73,6 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
}
}
- if (ret < 0)
- goto out;
-
/*
* fill in reserved space with some buffer details
* (eg. version, buffer size, number of measurements)
@@ -94,7 +92,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
*buffer_size = ima_kexec_file.count;
*buffer = ima_kexec_file.buf;
-out:
+
return ret;
}
@@ -112,9 +110,8 @@ void ima_add_kexec_buffer(struct kimage *image)
unsigned long binary_runtime_size;
/* use more understandable variable names than defined in kbuf */
+ size_t kexec_buffer_size = 0;
void *kexec_buffer = NULL;
- size_t kexec_buffer_size;
- size_t kexec_segment_size;
int ret;
/*
@@ -139,13 +136,6 @@ void ima_add_kexec_buffer(struct kimage *image)
return;
}
- ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
- kexec_segment_size);
- if (!kexec_buffer) {
- pr_err("Not enough memory for the kexec measurement buffer.\n");
- return;
- }
-
kbuf.buffer = kexec_buffer;
kbuf.bufsz = kexec_buffer_size;
kbuf.memsz = kexec_segment_size;
@@ -173,7 +163,32 @@ void ima_add_kexec_buffer(struct kimage *image)
static int ima_update_kexec_buffer(struct notifier_block *self,
unsigned long action, void *data)
{
- return NOTIFY_OK;
+ size_t buf_size = 0;
+ int ret = NOTIFY_OK;
+ void *buf = NULL;
+
+ if (!kexec_in_progress) {
+ pr_info("No kexec in progress.\n");
+ return ret;
+ }
+
+ if (!ima_kexec_buffer) {
+ pr_err("Kexec buffer not set.\n");
+ return ret;
+ }
+
+ ret = ima_dump_measurement_list(&buf_size, &buf, kexec_segment_size);
+
+ if (ret)
+ pr_err("Dump measurements failed. Error:%d\n", ret);
+
+ if (buf_size != 0)
+ memcpy(ima_kexec_buffer, buf, buf_size);
+
+ kimage_unmap_segment(ima_kexec_buffer);
+ ima_kexec_buffer = NULL;
+
+ return ret;
}
static struct notifier_block update_buffer_nb = {
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute
2025-04-02 12:47 ` [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute steven chen
@ 2025-04-08 16:17 ` Mimi Zohar
2025-04-10 14:15 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 16:17 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> ima_dump_measurement_list() is called during kexec 'load', which may
> result in loss of IMA measurements during kexec soft reboot. Due to
> missed measurements that only occurred after kexec 'load', this function
> needs to be called during kexec 'execute'.
Re-use the motivation from 5/9 (with tweak):
The IMA log is currently copied to the new kernel during kexec 'load' using
ima_dump_measurement_list(). However, the IMA measurement list copied at kexec
'load' may result in loss of IMA measurements records that only occurred after
the kexec 'load'.
And finish the paragraph with:
Move the IMA measurement list log copy from kexec 'load' to 'execute'.
>
> Make the kexec_segment_size variable a local static variable within the
> file, so it can be accessed during both kexec 'load' and 'execute'.
>
> Implement the kexec_post_load() function to be invoked after the new kernel
> image has been loaded for kexec. Instead of calling machine_kexec_post_load()
> directly from the kexec_file_load() syscall, call kexec_post_load(), which in
> turn calls machine_kexec_post_load() to maintain the original image processing.
Define kexec_post_load() as a wrapper for calling ima_kexec_post_load() and
machine_kexec_post_load(). Replace the existing direct call to
machine_kexec_post_load() with kexec_post_load().
>
> Invoke ima_kexec_post_load() within the kexec_post_load() API only for kexec
> soft reboot scenarios, excluding KEXEC_FILE_ON_CRASH.
"Don't call ima_kexec_post_load() on KEXEC_FILE_ON_CRASH" would be listed in the
Changelog if it changed, not here in the patch description. Please remove.
>
> Register a reboot notifier for the ima_update_kexec_buffer() API within
> ima_kexec_post_load() to ensure it is called upon receiving a reboot
> notification.
Registering the reboot notifier was done in "[PATCH v11 5/9] ima: kexec: define
functions to copy IMA log at soft boot", not here. Please remove.
>
> Move the ima_dump_measurement_list() call from ima_add_kexec_buffer() to
> ima_update_kexec_buffer() to copy the IMA log at the kexec 'execute' stage.
This information was already stated in the first paragraph as part of the
motivation for the patch. Please remove.
>
> When there is insufficient memory to copy all the measurement logs, copy as
> much of the measurement list as possible.
Is this comment still applicable to this patch?
Please review your patch descriptions before posting, making sure that
everything is still applicable.
thanks,
Mimi
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Cc: Eric Biederman <ebiederm@xmission.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: Dave Young <dyoung@redhat.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute
2025-04-08 16:17 ` Mimi Zohar
@ 2025-04-10 14:15 ` steven chen
0 siblings, 0 replies; 35+ messages in thread
From: steven chen @ 2025-04-10 14:15 UTC (permalink / raw)
To: Mimi Zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On 4/8/2025 9:17 AM, Mimi Zohar wrote:
> On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
>> ima_dump_measurement_list() is called during kexec 'load', which may
>> result in loss of IMA measurements during kexec soft reboot. Due to
>> missed measurements that only occurred after kexec 'load', this function
>> needs to be called during kexec 'execute'.
> Re-use the motivation from 5/9 (with tweak):
>
> The IMA log is currently copied to the new kernel during kexec 'load' using
> ima_dump_measurement_list(). However, the IMA measurement list copied at kexec
> 'load' may result in loss of IMA measurements records that only occurred after
> the kexec 'load'.
>
> And finish the paragraph with:
> Move the IMA measurement list log copy from kexec 'load' to 'execute'.
>
>> Make the kexec_segment_size variable a local static variable within the
>> file, so it can be accessed during both kexec 'load' and 'execute'.
>>
>> Implement the kexec_post_load() function to be invoked after the new kernel
>> image has been loaded for kexec. Instead of calling machine_kexec_post_load()
>> directly from the kexec_file_load() syscall, call kexec_post_load(), which in
>> turn calls machine_kexec_post_load() to maintain the original image processing.
> Define kexec_post_load() as a wrapper for calling ima_kexec_post_load() and
> machine_kexec_post_load(). Replace the existing direct call to
> machine_kexec_post_load() with kexec_post_load().
>
>>
>> Invoke ima_kexec_post_load() within the kexec_post_load() API only for kexec
>> soft reboot scenarios, excluding KEXEC_FILE_ON_CRASH.
> "Don't call ima_kexec_post_load() on KEXEC_FILE_ON_CRASH" would be listed in the
> Changelog if it changed, not here in the patch description. Please remove.
>
>>
>> Register a reboot notifier for the ima_update_kexec_buffer() API within
>> ima_kexec_post_load() to ensure it is called upon receiving a reboot
>> notification.
> Registering the reboot notifier was done in "[PATCH v11 5/9] ima: kexec: define
> functions to copy IMA log at soft boot", not here. Please remove.
>
>>
>> Move the ima_dump_measurement_list() call from ima_add_kexec_buffer() to
>> ima_update_kexec_buffer() to copy the IMA log at the kexec 'execute' stage.
> This information was already stated in the first paragraph as part of the
> motivation for the patch. Please remove.
>
>>
>> When there is insufficient memory to copy all the measurement logs, copy as
>> much of the measurement list as possible.
> Is this comment still applicable to this patch?
>
> Please review your patch descriptions before posting, making sure that
> everything is still applicable.
>
> thanks,
>
> Mimi
Hi Mimi,
Thanks for your comments. I will update in next version.
Steven
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Cc: Eric Biederman <ebiederm@xmission.com>
>> Cc: Baoquan He <bhe@redhat.com>
>> Cc: Vivek Goyal <vgoyal@redhat.com>
>> Cc: Dave Young <dyoung@redhat.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 7/9] ima: verify if the segment size has changed
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (5 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 6/9] ima: kexec: move IMA log copy from kexec load to execute steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 3:54 ` Baoquan He
2025-04-08 14:22 ` Mimi Zohar
2025-04-02 12:47 ` [PATCH v11 8/9] ima: make the kexec extra memory configurable steven chen
2025-04-02 12:47 ` [PATCH v11 9/9] ima: measure kexec load and exec events as critical data steven chen
8 siblings, 2 replies; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
kexec 'load' may be called multiple times. Free and realloc the buffer
only if the segment_size is changed from the previous kexec 'load' call.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima_kexec.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 5c3b3e0b2186..ed867734ee70 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -33,6 +33,14 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
static int ima_alloc_kexec_file_buf(size_t segment_size)
{
+ /*
+ * kexec 'load' may be called multiple times.
+ * Free and realloc the buffer only if the segment_size is
+ * changed from the previous kexec 'load' call.
+ */
+ if (ima_kexec_file.buf && ima_kexec_file.size == segment_size)
+ goto out;
+
ima_free_kexec_file_buf(&ima_kexec_file);
/* segment size can't change between kexec load and execute */
@@ -41,6 +49,8 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
return -ENOMEM;
ima_kexec_file.size = segment_size;
+
+out:
ima_kexec_file.read_pos = 0;
ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 7/9] ima: verify if the segment size has changed
2025-04-02 12:47 ` [PATCH v11 7/9] ima: verify if the segment size has changed steven chen
@ 2025-04-08 3:54 ` Baoquan He
2025-04-08 14:22 ` Mimi Zohar
1 sibling, 0 replies; 35+ messages in thread
From: Baoquan He @ 2025-04-08 3:54 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/02/25 at 05:47am, steven chen wrote:
> kexec 'load' may be called multiple times. Free and realloc the buffer
> only if the segment_size is changed from the previous kexec 'load' call.
This is a great example demonstrating how patch is nicely split. A
reasonable unit including reasonable code change and log.
Acked-by: Baoquan He <bhe@redhat.com>
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> security/integrity/ima/ima_kexec.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 5c3b3e0b2186..ed867734ee70 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -33,6 +33,14 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
>
> static int ima_alloc_kexec_file_buf(size_t segment_size)
> {
> + /*
> + * kexec 'load' may be called multiple times.
> + * Free and realloc the buffer only if the segment_size is
> + * changed from the previous kexec 'load' call.
> + */
> + if (ima_kexec_file.buf && ima_kexec_file.size == segment_size)
> + goto out;
> +
> ima_free_kexec_file_buf(&ima_kexec_file);
>
> /* segment size can't change between kexec load and execute */
> @@ -41,6 +49,8 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
> return -ENOMEM;
>
> ima_kexec_file.size = segment_size;
> +
> +out:
> ima_kexec_file.read_pos = 0;
> ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 7/9] ima: verify if the segment size has changed
2025-04-02 12:47 ` [PATCH v11 7/9] ima: verify if the segment size has changed steven chen
2025-04-08 3:54 ` Baoquan He
@ 2025-04-08 14:22 ` Mimi Zohar
1 sibling, 0 replies; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 14:22 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> kexec 'load' may be called multiple times. Free and realloc the buffer
> only if the segment_size is changed from the previous kexec 'load' call.
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
Thanks, Steven.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (6 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 7/9] ima: verify if the segment size has changed steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-10 9:54 ` Baoquan He
2025-04-02 12:47 ` [PATCH v11 9/9] ima: measure kexec load and exec events as critical data steven chen
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The extra memory allocated for carrying the IMA measurement list across
kexec is hard-coded as half a PAGE. Make it configurable.
Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
extra memory (in kb) to be allocated for IMA measurements added during
kexec soft reboot. Ensure the default value of the option is set such
that extra half a page of memory for additional measurements is allocated
for the additional measurements.
Update ima_add_kexec_buffer() function to allocate memory based on the
Kconfig option value, rather than the currently hard-coded one.
Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
security/integrity/ima/Kconfig | 10 ++++++++++
security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 475c32615006..d73c96c3c1c9 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
help
This option disables htable to allow measurement of duplicate records.
+config IMA_KEXEC_EXTRA_MEMORY_KB
+ int "Extra memory for IMA measurements added during kexec soft reboot"
+ depends on IMA_KEXEC
+ default 0
+ help
+ IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
+ allocated (in kb) for IMA measurements added during kexec soft reboot.
+ If set to the default value of 0, an extra half page of memory for those
+ additional measurements will be allocated.
+
endif
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index ed867734ee70..d1c9d369ba08 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -118,6 +118,7 @@ void ima_add_kexec_buffer(struct kimage *image)
.buf_min = 0, .buf_max = ULONG_MAX,
.top_down = true };
unsigned long binary_runtime_size;
+ unsigned long extra_memory;
/* use more understandable variable names than defined in kbuf */
size_t kexec_buffer_size = 0;
@@ -125,15 +126,20 @@ void ima_add_kexec_buffer(struct kimage *image)
int ret;
/*
- * Reserve an extra half page of memory for additional measurements
- * added during the kexec load.
+ * Reserve extra memory for measurements added during kexec.
*/
- binary_runtime_size = ima_get_binary_runtime_size();
+ if (CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB <= 0)
+ extra_memory = PAGE_SIZE / 2;
+ else
+ extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
+
+ binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
+
if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
kexec_segment_size = ULONG_MAX;
else
- kexec_segment_size = ALIGN(ima_get_binary_runtime_size() +
- PAGE_SIZE / 2, PAGE_SIZE);
+ kexec_segment_size = ALIGN(binary_runtime_size, PAGE_SIZE);
+
if ((kexec_segment_size == ULONG_MAX) ||
((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
pr_err("Binary measurement list too large.\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-02 12:47 ` [PATCH v11 8/9] ima: make the kexec extra memory configurable steven chen
@ 2025-04-10 9:54 ` Baoquan He
2025-04-10 16:59 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Baoquan He @ 2025-04-10 9:54 UTC (permalink / raw)
To: steven chen
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 04/02/25 at 05:47am, steven chen wrote:
> The extra memory allocated for carrying the IMA measurement list across
> kexec is hard-coded as half a PAGE. Make it configurable.
>
> Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
> extra memory (in kb) to be allocated for IMA measurements added during
> kexec soft reboot. Ensure the default value of the option is set such
> that extra half a page of memory for additional measurements is allocated
> for the additional measurements.
>
> Update ima_add_kexec_buffer() function to allocate memory based on the
> Kconfig option value, rather than the currently hard-coded one.
>
> Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> security/integrity/ima/Kconfig | 10 ++++++++++
> security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 475c32615006..d73c96c3c1c9 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
> help
> This option disables htable to allow measurement of duplicate records.
>
> +config IMA_KEXEC_EXTRA_MEMORY_KB
> + int "Extra memory for IMA measurements added during kexec soft reboot"
> + depends on IMA_KEXEC
> + default 0
Usually a new Kconfig item which accepts a range should define the range
boundary, otherwise it's not clear to people how large or how small it
can be set. For example, can I set it as value of 1<<40? We should at
least estimate a possible upper limit for it for other people's
reference. My personal opinion.
The rest looks good to me.
> + help
> + IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
> + allocated (in kb) for IMA measurements added during kexec soft reboot.
> + If set to the default value of 0, an extra half page of memory for those
> + additional measurements will be allocated.
> +
> endif
...snip...
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-10 9:54 ` Baoquan He
@ 2025-04-10 16:59 ` steven chen
2025-04-10 18:04 ` Mimi Zohar
0 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-10 16:59 UTC (permalink / raw)
To: Baoquan He
Cc: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On 4/10/2025 2:54 AM, Baoquan He wrote:
> On 04/02/25 at 05:47am, steven chen wrote:
>> The extra memory allocated for carrying the IMA measurement list across
>> kexec is hard-coded as half a PAGE. Make it configurable.
>>
>> Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
>> extra memory (in kb) to be allocated for IMA measurements added during
>> kexec soft reboot. Ensure the default value of the option is set such
>> that extra half a page of memory for additional measurements is allocated
>> for the additional measurements.
>>
>> Update ima_add_kexec_buffer() function to allocate memory based on the
>> Kconfig option value, rather than the currently hard-coded one.
>>
>> Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>> ---
>> security/integrity/ima/Kconfig | 10 ++++++++++
>> security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>> index 475c32615006..d73c96c3c1c9 100644
>> --- a/security/integrity/ima/Kconfig
>> +++ b/security/integrity/ima/Kconfig
>> @@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
>> help
>> This option disables htable to allow measurement of duplicate records.
>>
>> +config IMA_KEXEC_EXTRA_MEMORY_KB
>> + int "Extra memory for IMA measurements added during kexec soft reboot"
>> + depends on IMA_KEXEC
>> + default 0
> Usually a new Kconfig item which accepts a range should define the range
> boundary, otherwise it's not clear to people how large or how small it
> can be set. For example, can I set it as value of 1<<40? We should at
> least estimate a possible upper limit for it for other people's
> reference. My personal opinion.
Hi Baoquan,
How about I set range 2-40? Default set as 2, same as the fixed setting.
Thanks,
Steven
> The rest looks good to me.
>
>
>> + help
>> + IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
>> + allocated (in kb) for IMA measurements added during kexec soft reboot.
>> + If set to the default value of 0, an extra half page of memory for those
>> + additional measurements will be allocated.
>> +
>> endif
> ...snip...
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-10 16:59 ` steven chen
@ 2025-04-10 18:04 ` Mimi Zohar
2025-04-10 18:49 ` steven chen
0 siblings, 1 reply; 35+ messages in thread
From: Mimi Zohar @ 2025-04-10 18:04 UTC (permalink / raw)
To: steven chen, Baoquan He, Lakshmi Ramasubramanian
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, nramas,
James.Bottomley, vgoyal, dyoung
On Thu, 2025-04-10 at 09:59 -0700, steven chen wrote:
> On 4/10/2025 2:54 AM, Baoquan He wrote:
> > On 04/02/25 at 05:47am, steven chen wrote:
> > > The extra memory allocated for carrying the IMA measurement list across
> > > kexec is hard-coded as half a PAGE. Make it configurable.
> > >
> > > Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
> > > extra memory (in kb) to be allocated for IMA measurements added during
> > > kexec soft reboot. Ensure the default value of the option is set such
> > > that extra half a page of memory for additional measurements is allocated
> > > for the additional measurements.
> > >
> > > Update ima_add_kexec_buffer() function to allocate memory based on the
> > > Kconfig option value, rather than the currently hard-coded one.
> > >
> > > Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
> > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > ---
> > > security/integrity/ima/Kconfig | 10 ++++++++++
> > > security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
> > > 2 files changed, 21 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > > index 475c32615006..d73c96c3c1c9 100644
> > > --- a/security/integrity/ima/Kconfig
> > > +++ b/security/integrity/ima/Kconfig
> > > @@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
> > > help
> > > This option disables htable to allow measurement of duplicate records.
> > >
> > > +config IMA_KEXEC_EXTRA_MEMORY_KB
> > > + int "Extra memory for IMA measurements added during kexec soft reboot"
> > > + depends on IMA_KEXEC
> > > + default 0
> > Usually a new Kconfig item which accepts a range should define the range
> > boundary, otherwise it's not clear to people how large or how small it
> > can be set. For example, can I set it as value of 1<<40? We should at
> > least estimate a possible upper limit for it for other people's
> > reference. My personal opinion.
>
> Hi Baoquan,
>
> How about I set range 2-40? Default set as 2, same as the fixed setting.
0, the current default, sets the "extra" memory to the existing "extra half a
page of memory for the additional measurements". For backwards compatibility,
please do not change this.
The requirement for a larger "extra" measurement is coming from Microsoft. If
this isn't any longer a requirement, we could drop this patch. Lakshmi, do you
have any thoughts on this?
thanks,
Mimi
>
> > The rest looks good to me.
> >
> >
> > > + help
> > > + IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
> > > + allocated (in kb) for IMA measurements added during kexec soft reboot.
> > > + If set to the default value of 0, an extra half page of memory for those
> > > + additional measurements will be allocated.
> > > +
> > > endif
> > ...snip...
>
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-10 18:04 ` Mimi Zohar
@ 2025-04-10 18:49 ` steven chen
2025-04-10 19:47 ` Mimi Zohar
0 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-10 18:49 UTC (permalink / raw)
To: Mimi Zohar, Baoquan He, Lakshmi Ramasubramanian
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, James.Bottomley,
vgoyal, dyoung
On 4/10/2025 11:04 AM, Mimi Zohar wrote:
> On Thu, 2025-04-10 at 09:59 -0700, steven chen wrote:
>> On 4/10/2025 2:54 AM, Baoquan He wrote:
>>> On 04/02/25 at 05:47am, steven chen wrote:
>>>> The extra memory allocated for carrying the IMA measurement list across
>>>> kexec is hard-coded as half a PAGE. Make it configurable.
>>>>
>>>> Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
>>>> extra memory (in kb) to be allocated for IMA measurements added during
>>>> kexec soft reboot. Ensure the default value of the option is set such
>>>> that extra half a page of memory for additional measurements is allocated
>>>> for the additional measurements.
>>>>
>>>> Update ima_add_kexec_buffer() function to allocate memory based on the
>>>> Kconfig option value, rather than the currently hard-coded one.
>>>>
>>>> Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
>>>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>>>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>>>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>>>> ---
>>>> security/integrity/ima/Kconfig | 10 ++++++++++
>>>> security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
>>>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>>>> index 475c32615006..d73c96c3c1c9 100644
>>>> --- a/security/integrity/ima/Kconfig
>>>> +++ b/security/integrity/ima/Kconfig
>>>> @@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
>>>> help
>>>> This option disables htable to allow measurement of duplicate records.
>>>>
>>>> +config IMA_KEXEC_EXTRA_MEMORY_KB
>>>> + int "Extra memory for IMA measurements added during kexec soft reboot"
>>>> + depends on IMA_KEXEC
>>>> + default 0
>>> Usually a new Kconfig item which accepts a range should define the range
>>> boundary, otherwise it's not clear to people how large or how small it
>>> can be set. For example, can I set it as value of 1<<40? We should at
>>> least estimate a possible upper limit for it for other people's
>>> reference. My personal opinion.
>> Hi Baoquan,
>>
>> How about I set range 2-40? Default set as 2, same as the fixed setting.
> 0, the current default, sets the "extra" memory to the existing "extra half a
> page of memory for the additional measurements". For backwards compatibility,
> please do not change this.
>
> The requirement for a larger "extra" measurement is coming from Microsoft. If
> this isn't any longer a requirement, we could drop this patch. Lakshmi, do you
> have any thoughts on this?
>
> thanks,
>
> Mimi
How about the range set as 0-40 and the default as 0?
We (Microsoft) are ok with 0 as the default.
Thanks
>>> The rest looks good to me.
>>>
>>>
>>>> + help
>>>> + IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
>>>> + allocated (in kb) for IMA measurements added during kexec soft reboot.
>>>> + If set to the default value of 0, an extra half page of memory for those
>>>> + additional measurements will be allocated.
>>>> +
>>>> endif
>>> ...snip...
>>
>>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v11 8/9] ima: make the kexec extra memory configurable
2025-04-10 18:49 ` steven chen
@ 2025-04-10 19:47 ` Mimi Zohar
0 siblings, 0 replies; 35+ messages in thread
From: Mimi Zohar @ 2025-04-10 19:47 UTC (permalink / raw)
To: steven chen, Baoquan He, Lakshmi Ramasubramanian
Cc: stefanb, roberto.sassu, roberto.sassu, eric.snowberg, ebiederm,
paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel, madvenka, James.Bottomley,
vgoyal, dyoung
On Thu, 2025-04-10 at 11:49 -0700, steven chen wrote:
> On 4/10/2025 11:04 AM, Mimi Zohar wrote:
> > On Thu, 2025-04-10 at 09:59 -0700, steven chen wrote:
> > > On 4/10/2025 2:54 AM, Baoquan He wrote:
> > > > On 04/02/25 at 05:47am, steven chen wrote:
> > > > > The extra memory allocated for carrying the IMA measurement list across
> > > > > kexec is hard-coded as half a PAGE. Make it configurable.
> > > > >
> > > > > Define a Kconfig option, IMA_KEXEC_EXTRA_MEMORY_KB, to configure the
> > > > > extra memory (in kb) to be allocated for IMA measurements added during
> > > > > kexec soft reboot. Ensure the default value of the option is set such
> > > > > that extra half a page of memory for additional measurements is allocated
> > > > > for the additional measurements.
> > > > >
> > > > > Update ima_add_kexec_buffer() function to allocate memory based on the
> > > > > Kconfig option value, rather than the currently hard-coded one.
> > > > >
> > > > > Suggested-by: Stefan Berger <stefanb@linux.ibm.com>
> > > > > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> > > > > Signed-off-by: steven chen <chenste@linux.microsoft.com>
> > > > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > > > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > ---
> > > > > security/integrity/ima/Kconfig | 10 ++++++++++
> > > > > security/integrity/ima/ima_kexec.c | 16 +++++++++++-----
> > > > > 2 files changed, 21 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > > > > index 475c32615006..d73c96c3c1c9 100644
> > > > > --- a/security/integrity/ima/Kconfig
> > > > > +++ b/security/integrity/ima/Kconfig
> > > > > @@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
> > > > > help
> > > > > This option disables htable to allow measurement of duplicate records.
> > > > >
> > > > > +config IMA_KEXEC_EXTRA_MEMORY_KB
> > > > > + int "Extra memory for IMA measurements added during kexec soft reboot"
> > > > > + depends on IMA_KEXEC
> > > > > + default 0
> > > > Usually a new Kconfig item which accepts a range should define the range
> > > > boundary, otherwise it's not clear to people how large or how small it
> > > > can be set. For example, can I set it as value of 1<<40? We should at
> > > > least estimate a possible upper limit for it for other people's
> > > > reference. My personal opinion.
> > > Hi Baoquan,
> > >
> > > How about I set range 2-40? Default set as 2, same as the fixed setting.
> > 0, the current default, sets the "extra" memory to the existing "extra half a
> > page of memory for the additional measurements". For backwards compatibility,
> > please do not change this.
> >
> > The requirement for a larger "extra" measurement is coming from Microsoft. If
> > this isn't any longer a requirement, we could drop this patch. Lakshmi, do you
> > have any thoughts on this?
>
> How about the range set as 0-40 and the default as 0?
>
> We (Microsoft) are ok with 0 as the default.
Thanks, fine.
>
> Thanks
>
> > > > The rest looks good to me.
> > > >
> > > >
> > > > > + help
> > > > > + IMA_KEXEC_EXTRA_MEMORY_KB determines the extra memory to be
> > > > > + allocated (in kb) for IMA measurements added during kexec soft reboot.
> > > > > + If set to the default value of 0, an extra half page of memory for those
> > > > > + additional measurements will be allocated.
> > > > > +
> > > > > endif
> > > > ...snip...
> > >
> > >
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v11 9/9] ima: measure kexec load and exec events as critical data
2025-04-02 12:47 [PATCH v11 0/9] ima: kexec: measure events between kexec load and execute steven chen
` (7 preceding siblings ...)
2025-04-02 12:47 ` [PATCH v11 8/9] ima: make the kexec extra memory configurable steven chen
@ 2025-04-02 12:47 ` steven chen
2025-04-08 16:31 ` Mimi Zohar
8 siblings, 1 reply; 35+ messages in thread
From: steven chen @ 2025-04-02 12:47 UTC (permalink / raw)
To: zohar, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
The amount of memory allocated at kexec load, even with the extra memory
allocated, might not be large enough for the entire measurement list. The
indeterminate interval between kexec 'load' and 'execute' could exacerbate
this problem.
Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
measured as critical data at kexec 'load' and 'execute' respectively.
Report the allocated kexec segment size, IMA binary log size and the
runtime measurements count as part of those events.
These events, and the values reported through them, serve as markers in
the IMA log to verify the IMA events are captured during kexec soft
reboot. The presence of a 'kexec_load' event in between the last two
'boot_aggregate' events in the IMA log implies this is a kexec soft
reboot, and not a cold-boot. And the absence of 'kexec_execute' event
after kexec soft reboot implies missing events in that window which
results in inconsistency with TPM PCR quotes, necessitating a cold boot
for a successful remote attestation.
These critical data events are displayed as hex encoded ascii in the
ascii_runtime_measurement_list. Verifying the critical data hash requires
calculating the hash of the decoded ascii string.
For example, to verify the 'kexec_load' data hash:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
| grep kexec_load | cut -d' ' -f 6 | xxd -r -p | sha256sum
To verify the 'kexec_execute' data hash:
sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
| grep kexec_execute | cut -d' ' -f 6 | xxd -r -p | sha256sum
Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
security/integrity/ima/ima.h | 6 ++++++
security/integrity/ima/ima_kexec.c | 21 +++++++++++++++++++++
security/integrity/ima/ima_queue.c | 5 +++++
3 files changed, 32 insertions(+)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 24d09ea91b87..34815baf5e21 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -240,6 +240,12 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
unsigned long flags, bool create);
#endif
+#ifdef CONFIG_IMA_KEXEC
+void ima_measure_kexec_event(const char *event_name);
+#else
+static inline void ima_measure_kexec_event(const char *event_name) {}
+#endif
+
/*
* The default binary_runtime_measurements list format is defined as the
* platform native format. The canonical format is defined as little-endian.
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index d1c9d369ba08..38cb2500f4c3 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -17,6 +17,8 @@
#include "ima.h"
#ifdef CONFIG_IMA_KEXEC
+#define IMA_KEXEC_EVENT_LEN 256
+
static bool ima_kexec_update_registered;
static struct seq_file ima_kexec_file;
static size_t kexec_segment_size;
@@ -31,6 +33,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf)
sf->count = 0;
}
+void ima_measure_kexec_event(const char *event_name)
+{
+ char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
+ size_t buf_size = 0;
+ long len;
+ int n;
+
+ buf_size = ima_get_binary_runtime_size();
+ len = atomic_long_read(&ima_htable.len);
+
+ n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
+ "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
+ "ima_runtime_measurements_count=%ld;",
+ kexec_segment_size, buf_size, len);
+
+ ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, n, false, NULL, 0);
+}
+
static int ima_alloc_kexec_file_buf(size_t segment_size)
{
/*
@@ -53,6 +73,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
out:
ima_kexec_file.read_pos = 0;
ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */
+ ima_measure_kexec_event("kexec_load");
return 0;
}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 83d53824aa98..590637e81ad1 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -241,6 +241,11 @@ static int ima_reboot_notifier(struct notifier_block *nb,
unsigned long action,
void *data)
{
+#ifdef CONFIG_IMA_KEXEC
+ if (action == SYS_RESTART && data && !strcmp(data, "kexec reboot"))
+ ima_measure_kexec_event("kexec_execute");
+#endif
+
ima_measurements_suspend();
return NOTIFY_DONE;
--
2.25.1
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v11 9/9] ima: measure kexec load and exec events as critical data
2025-04-02 12:47 ` [PATCH v11 9/9] ima: measure kexec load and exec events as critical data steven chen
@ 2025-04-08 16:31 ` Mimi Zohar
0 siblings, 0 replies; 35+ messages in thread
From: Mimi Zohar @ 2025-04-08 16:31 UTC (permalink / raw)
To: steven chen, stefanb, roberto.sassu, roberto.sassu, eric.snowberg,
ebiederm, paul, code, bauermann, linux-integrity, kexec,
linux-security-module, linux-kernel
Cc: madvenka, nramas, James.Bottomley, bhe, vgoyal, dyoung
On Wed, 2025-04-02 at 05:47 -0700, steven chen wrote:
> The amount of memory allocated at kexec load, even with the extra memory
> allocated, might not be large enough for the entire measurement list. The
> indeterminate interval between kexec 'load' and 'execute' could exacerbate
> this problem.
>
> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
> measured as critical data at kexec 'load' and 'execute' respectively.
> Report the allocated kexec segment size, IMA binary log size and the
> runtime measurements count as part of those events.
>
> These events, and the values reported through them, serve as markers in
> the IMA log to verify the IMA events are captured during kexec soft
> reboot. The presence of a 'kexec_load' event in between the last two
> 'boot_aggregate' events in the IMA log implies this is a kexec soft
> reboot, and not a cold-boot. And the absence of 'kexec_execute' event
> after kexec soft reboot implies missing events in that window which
> results in inconsistency with TPM PCR quotes, necessitating a cold boot
> for a successful remote attestation.
>
> These critical data events are displayed as hex encoded ascii in the
> ascii_runtime_measurement_list. Verifying the critical data hash requires
> calculating the hash of the decoded ascii string.
>
> For example, to verify the 'kexec_load' data hash:
>
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
> > grep kexec_load | cut -d' ' -f 6 | xxd -r -p | sha256sum
>
>
> To verify the 'kexec_execute' data hash:
>
> sudo cat /sys/kernel/security/integrity/ima/ascii_runtime_measurements
> > grep kexec_execute | cut -d' ' -f 6 | xxd -r -p | sha256sum
>
> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Thanks, Steven.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply [flat|nested] 35+ messages in thread