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@xxxxxxxxxxxxxxxxxxx>
---
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
