Adding Eric to cc.
On 7/7/23 06:01, Mimi Zohar wrote:
Hi Tushar,
The function to "ima: allocate buffer at kexec load to hold ima
measurements" already exists. Please update the Subject line to
indicate increasing the IMA kexec buffer size.
On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote:
The IMA subsystem needs a dedicated mechanism to reserve extra memory for
measurements added between the kexec 'load' and kexec 'execute'.
What is a "dedicated mechanism?". Either start the sentence with
"reserve ..." or completely remove it.
Will do.
Update ima_add_kexec_buffer to allocate a buffer of a sufficient size
taking ima binary runtime measurements size, size of ima_kexec_hdr, and
IMA_KEXEC_EXTRA_SIZE into account. Adjust the kexec_segment_size to align
to the PAGE_SIZE. Call ima_allocate_buf_at_kexec_load() to allocate the
buffer.
The size of the ima_kexec_hdr kind of gets lost in the amount of
additional memory being allocated, but sure it's a nice clean up.
Thanks. :)
This patch assumes the extra space defined (IMA_KEXEC_EXTRA_SIZE) is
sufficient to handle the additional measurements. This should be as per
the system requirements and based on the number of additional measurements
expected during the window from kexec 'load' to kexec 'execute'.
Should the extra amount of memory be hard coded?
You are right. As you suggested below, defining the extra amount in
Kconfig is a cleaner approach.
Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_kexec.c | 21 ++++++++++-----------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c29db699c996..2ffda9449b9b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -43,6 +43,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
#define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
+#define IMA_KEXEC_EXTRA_SIZE (16 * PAGE_SIZE)
Not all IMA policies require extra memory. Define and use a new IMA
Kconfig to set the amount of extra memory.
Sure. Will do.
/* current content of the policy */
extern int ima_policy_flag;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 858b67689701..7deb8df31485 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -188,31 +188,30 @@ void ima_add_kexec_buffer(struct kimage *image)
/* use more understandable variable names than defined in kbuf */
void *kexec_buffer = NULL;
size_t kexec_buffer_size;
- size_t kexec_segment_size;
int ret;
/*
- * Reserve an extra half page of memory for additional measurements
- * added during the kexec load.
+ * Reserve extra memory for measurements added in the window from
+ * kexec 'load' to kexec 'execute'.
*/
- binary_runtime_size = ima_get_binary_runtime_size();
+ binary_runtime_size = ima_get_binary_runtime_size() +
+ sizeof(struct ima_kexec_hdr) +
+ IMA_KEXEC_EXTRA_SIZE;
+
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");
return;
}
- ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
- kexec_segment_size);
Removing the call to ima_dump_measurement_list here is not bisect safe.
Agreed. Will take care of this based on if we end up using just
ima_dump_measurement_list(), or combination of
ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute().
~Tushar
- if (!kexec_buffer) {
- pr_err("Not enough memory for the kexec measurement buffer.\n");
+ ret = ima_allocate_buf_at_kexec_load();
+ if (ret < 0)
return;
- }
kbuf.buffer = kexec_buffer;
kbuf.bufsz = kexec_buffer_size;
