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.
> 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.
> 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?
> 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.
> /* 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.
> - 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;
--
thanks,
Mimi
