Re: [PATCH v2 6/7] ima: make the memory for events between kexec load and exec configurable

From: Tushar Sugandhi <tusharsu@linux.microsoft.com>
To: Stefan Berger <stefanb@linux.ibm.com>,
	zohar@linux.ibm.com, ebiederm@xmission.com, noodles@fb.com,
	bauermann@kolabnow.com, kexec@lists.infradead.org,
	linux-integrity@vger.kernel.org
Cc: code@tyhicks.com, nramas@linux.microsoft.com, paul@paul-moore.com
Subject: Re: [PATCH v2 6/7] ima: make the memory for events between kexec load and exec configurable
Date: Fri, 20 Oct 2023 13:39:37 -0700	[thread overview]
Message-ID: <55585644-2170-d462-4d64-ca3a963b30fe@linux.microsoft.com> (raw)
In-Reply-To: <a8828617-a296-8498-5e1f-b75a089916e0@linux.ibm.com>

On 10/12/23 17:27, Stefan Berger wrote:
> 
> On 10/5/23 14:26, Tushar Sugandhi wrote:
>> IMA currently allocates half a PAGE_SIZE for the extra events that would
>> be measured between kexec 'load' and 'execute'.  Depending on the IMA
>> policy and the system state, that memory may not be sufficient to hold
>> the extra IMA events measured after kexec 'load'.  The memory
>> requirements vary from system to system and they should be configurable.
>>
>> Define a Kconfig option, IMA_KEXEC_EXTRA_PAGES, to configure the number
>> of extra pages to be allocated for IMA measurements added in the window
>> from kexec 'load' to kexec 'execute'.
>>
>> Update ima_add_kexec_buffer() function to allocate memory based on the
>> Kconfig option value, rather than the currently hardcoded one.
>>
>> Signed-off-by: Tushar Sugandhi<tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/Kconfig     |  9 +++++++++
>>   security/integrity/ima/ima_kexec.c | 13 ++++++++-----
>>   2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/security/integrity/ima/Kconfig 
>> b/security/integrity/ima/Kconfig
>> index 60a511c6b583..1b55cd2bcb36 100644
>> --- a/security/integrity/ima/Kconfig
>> +++ b/security/integrity/ima/Kconfig
>> @@ -338,3 +338,12 @@ config IMA_DISABLE_HTABLE
>>       default n
>>       help
>>          This option disables htable to allow measurement of duplicate 
>> records.
>> +
>> +config IMA_KEXEC_EXTRA_PAGES
>> +    int
>> +    depends on IMA && IMA_KEXEC
>> +    default 16
>> +    help
>> +      IMA_KEXEC_EXTRA_PAGES determines the number of extra
>> +      pages to be allocated for IMA measurements added in the
>> +      window from kexec 'load' to kexec 'execute'.
> 
> 
> On ppc64 a page is 64kb. I would ask for additional kb here.
> 
> 
Not sure I understand.  Do you mean I should make the default value of
the config IMA_KEXEC_EXTRA_PAGES 64 or something?

In code, I multiply the config value with PAGE_SIZE.  So more memory
would be allocated on ppc64 for given default config value. Could you
please clarify what change you are suggesting here?

+        binary_runtime_size = ima_get_binary_runtime_size() +
+                               sizeof(struct ima_kexec_hdr) +
+                               (CONFIG_IMA_KEXEC_EXTRA_PAGES *
PAGE_SIZE);

~Tushar

>> diff --git a/security/integrity/ima/ima_kexec.c 
>> b/security/integrity/ima/ima_kexec.c
>> index 13fbbb90319b..6cd5f46a7208 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -150,15 +150,18 @@ 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 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) +
>> +                  (CONFIG_IMA_KEXEC_EXTRA_PAGES * PAGE_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");