Re: [PATCH v2 1/7] ima: refactor ima_dump_measurement_list to move memory allocation to a separate function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/26/23 20:25, Mimi Zohar wrote:
On Thu, 2023-10-26 at 16:16 -0400, Mimi Zohar wrote:
Hi Tushar,

According to Documentation/process/submitting-patches.rst, the subject
line should be between 70-75 characters.

Perhaps something like "ima: define and call ima_alloc_kexec_buffer()".

On Thu, 2023-10-05 at 11:25 -0700, Tushar Sugandhi wrote:
IMA allocates memory and dumps the measurement during kexec soft reboot
as a single function call ima_dump_measurement_list().  It gets called
during kexec 'load' operation.  It results in the IMA measurements
between the window of kexec 'load' and 'execute' getting dropped when the
system boots into the new Kernel.  One of the kexec requirements is the
segment size cannot change between the 'load' and the 'execute'.
Therefore, to address this problem, ima_dump_measurement_list() needs
to be refactored to allocate the memory at kexec 'load', and dump the
measurements at kexec 'execute'.  The function that allocates the memory
should handle the scenario where the kexec load is called multiple times

The above pragraph is unnecessary.

Refactor ima_dump_measurement_list() to move the memory allocation part
to a separate function ima_alloc_kexec_buf() to allocate buffer of size
'kexec_segment_size' at kexec 'load'.  Make the local variables in
function ima_dump_measurement_list() global, so that they can be accessed
from ima_alloc_kexec_buf().  Make necessary changes to the function
ima_add_kexec_buffer() to call the above two functions.

Fix the wording based on the suggested changes below.

Signed-off-by: Tushar Sugandhi <tusharsu@xxxxxxxxxxxxxxxxxxx>

- Before re-posting this patch set, verify there aren't any
"checkpatch.pl --strict" issues.
- After applying each patch, compile the kernel and verify it still
works.

Doing this will detect whether or not the patch set is bisect safe.

I usually just do checkpatch.pl <.patch file>.
I didn't know about --strict and it's benefits.
Will do it going forward.


---
  security/integrity/ima/ima_kexec.c | 126 +++++++++++++++++++++--------
  1 file changed, 93 insertions(+), 33 deletions(-)

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 419dc405c831..307e07991865 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -15,61 +15,114 @@
  #include "ima.h"
#ifdef CONFIG_IMA_KEXEC
+struct seq_file ima_kexec_file;

Define "ima_kexec_file" as static since it only used in this file.
Since the variable does not need to be global, is there still a reason
for changing its name?   Minimize code change.

Adding "static" would make ima_kexec_file a global static variable.
Please ignore my comment about reverting the variable name change.

Mimi

Sure :)

~Tushar
...




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux