Re: [PATCH v3 6/7] ima: configure memory to log events between kexec load and execute

[Tushar Sugandhi <tusharsu@linux.microsoft.com>]

On 1/11/24 11:20, Stefan Berger wrote:
> 
> 
> On 1/11/24 13:13, Tushar Sugandhi wrote:
>>
>>
>> On 1/7/24 09:00, Mimi Zohar wrote:
>>> On Fri, 2024-01-05 at 12:20 -0800, Tushar Sugandhi wrote:
>>>>>> diff --git a/security/integrity/ima/Kconfig 
>>>>>> b/security/integrity/ima/Kconfig
>>>>>> index 60a511c6b583..8792b7aab768 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_MEMORY_KB
>>>>>> +    int
>>>>>> +    depends on IMA && IMA_KEXEC
>>>>>> +    default 64
>>>>>
>>>>> Since this isn't optional, the default should remain as a half page.
>>>>> Since a page is architecture specific, the default will need to be 
>>>>> arch
>>>>>    specific
>>>>>
>>>> It was a feedback from Stefan in the V2 of this series to convert it
>>>> from number of PAGES to KB.[1]
>>>>
>>>> But I can revert it to number of pages again.
>>>>
>>>> Also, making the default value as a fraction (1/2 page) feels weird for
>>>> a CONFIG variable.
>>>>
>>>> Is it ok to make the default value as one page rather than half page?
>>>
>>> The point is not whether the extra memory is specified in terms of 
>>> pages or KB.
>>> For backwards compatibility the existing default should be the same as
>>> previously.  This means the default needs to be architecture specific.b
>>> $ uname -m; getconf PAGESIZE
>>> x86_64
>>> 4096
>>> $ uname -m; getconf PAGESIZE
>>> ppc64le
>>> 65536
>>>
>>> For example:
>>>
>>> default 32 if PPC_64K_PAGES
>>> default 2
>>>
>> Ok. Thanks for the clarification.
>>
>>
>> Do we want to support only 64K or 4K as possible PAGE_SIZE values?
>> I spot checked a few architectures, there are scenarios where PAGE_SIZE
>> could be 8K, 16K, 128K, 256K etc. And of course mega pages with
>> PAGE_SIZE IN MBs (details below).
> 
> I would let the user specify the number of kilobytes to reserve and from 
> this you can conclude the page numbers:
> 
> needed_pages = KBs_TO_RESERVE / PAGE_SIZE
> if (KBs_TO_RESERVER % PAGE_SIZE)
>      needed_pages++;
> 
>     Stefan
Thanks Stefan.

But the issue here is about the default value,
not the user specified value.

Mimi is suggesting to keep the default value half-a-page,
to maintain backwards compatibility.

If we go with the KBs approach -
half-a-page translates to different KBs on different architectures.
And setting the right default value in KBs which would translate to
the desired half-a-page, on a given arch, inside the Kconfig seems
fragile (as I mentioned in the context of Option A in my previous
response.

And if we go with num_pages approach -
putting a fractional value (0.5) as a default in Kconfig seems to be non
trivial too.

Translating num_pages to KBs is trivial in code, but I think its
orthogonal to this conversation, since its about setting the desired 
arch specific default value in Kconfig.


Option A:
---------
config IMA_KEXEC_EXTRA_MEMORY_KB
     int
     depends on IMA && IMA_KEXEC
     default 128 if PAGE_SIZE_256KB
     default 32 if PPC_64K_PAGES || PAGE_SIZE_64KB || PARISC_PAGE_SIZE_16KB
     default 16 if PAGE_SIZE_32KB
     default 8 if PAGE_SIZE_16KB || ARC_PAGE_SIZE_16K || 
PARISC_PAGE_SIZE_16KB
     default 4 if PAGE_SIZE_8KB || ARC_PAGE_SIZE_8K
     default 2
       IMA_KEXEC_EXTRA_MEMORY_KB determines the number of extra
       memory (in KB) to be allocated for IMA measurements added
       during kexec soft-reboot.

Option B:
--------
config IMA_KEXEC_EXTRA_PAGES
     int
     depends on IMA && IMA_KEXEC
     default 1
     help
       IMA_KEXEC_EXTRA_PAGES determines the number of extra
       pages to be allocated for IMA measurements added during
       kexec soft-reboot.

~Tushar