[PATCH v4 6/7] ima: make the kexec extra memory configurable

[Stefan Berger]

On 1/23/24 16:19, Tushar Sugandhi wrote:
> Thanks again Stefan for taking a look.
> 
> On 1/23/24 11:02, Stefan Berger wrote:
>>
>>
>> On 1/22/24 13:38, Tushar Sugandhi wrote:
>>> The extra memory allocated for carrying the IMA measurement list across
>>> kexec is hardcoded 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
>>> to maintain backwards compatibility.
>>>
>>> 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 at linux.microsoft.com>
>>> ---
>>>   security/integrity/ima/Kconfig     | 11 +++++++++++
>>>   security/integrity/ima/ima_kexec.c | 15 ++++++++++-----
>>>   2 files changed, 21 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/security/integrity/ima/Kconfig 
>>> b/security/integrity/ima/Kconfig
>>> index 60a511c6b583..fc103288852b 100644
>>> --- a/security/integrity/ima/Kconfig
>>> +++ b/security/integrity/ima/Kconfig
>>> @@ -338,3 +338,14 @@ config IMA_DISABLE_HTABLE
>>>       default n
>>>       help
>>>          This option disables htable to allow measurement of 
>>> duplicate records.
>>> +
>>> +config IMA_KEXEC_EXTRA_MEMORY_KB
>>> +    int
>>> +    depends on IMA && 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, an extra half page of memory for
>>> +      additional measurements will be allocated to maintain backwards
>>> +      compatibility.
>>
>> Is there really an issue with 'backwards compatibility' that the user 
>> needs to know about ? From looking at the code it seems more important 
>> that a bit of additional memory is reserved now to fit the kexec 
>> 'load' and 'exec' critical data events but that's not 'backwards 
>> compatibility'.
>>
> I was contemplating how to put the right description in place 
> considering the conversation we had in v3 of this series[1].
> Without that context[1] default 0 could be equally confusing to the end 
> user.  With the phrase 'backwards compatibility', I was trying to 
> address the potential confusion around the default value 0 in the config 
> - that it represents half-a-page as default. And that particular value 
> choice ( half-a-page) is for backwards compatibility.
> You are right, I the user shouldn't care about it.  But I had to start 
> somewhere so that we can have this discussion on this thread. :)
> 
> Let me know how this description looks after removing the phrase 
> 'backwards compatibility':
> 
> " 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, an extra half a page of memory for those
>    additional measurements will be allocated."

Sounds good to me.
> 
> Lastly, do you want me to add suggested-by and/or reviewed-by tag to 
> this patch? Let me know. I will be happy to do so.

Either way is fine by me.

> 
> Thanks,
> Tushar
>