Re: [PATCH v3 1/7] ima: define and call ima_alloc_kexec_file_buf

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

Thanks Mimi for the feedback on V3.

Sorry for the late response, I was on vacation during the holidays.

Responses inline.

Happy new year! :)

On 12/20/23 08:13, Mimi Zohar wrote:
> On Fri, 2023-12-15 at 17:07 -0800, Tushar Sugandhi wrote:
>> Refactor ima_dump_measurement_list() to move the memory allocation part
>> to a separate function ima_alloc_kexec_file_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_file_buf().
> 
> Only ima_kexec_file is being moved and does not need to be global, but
> should be defined as local static.
> 

Will do.
I followed your comment on V2 of this series [1], and didn't make the
change last time. But making it local static scoped to this source file
(ima_kexec.c) makes sense.


[1] From v2 of this series
https://lore.kernel.org/all/2c9e3b71-5416-4336-82f1-cd78e26dd62e@linux.microsoft.com/
 >> 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
 >


>> Make necessary changes to the function
>> ima_add_kexec_buffer() to call the above two functions.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
>> ---
>>   security/integrity/ima/ima_kexec.c | 96 +++++++++++++++++++++---------
>>   1 file changed, 67 insertions(+), 29 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 419dc405c831..ae27101d0615 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -15,62 +15,93 @@
>>   #include "ima.h"
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>> +struct seq_file ima_kexec_file;
> 
> ^static.
> 
Will do.

>> +void ima_free_kexec_file_buf(struct seq_file *sf)
> 
> ^static
Will do.

~Tushar
> 
>> +{
>> +	vfree(sf->buf);
>> +	sf->buf = NULL;
>> +	sf->size = 0;
>> +	sf->read_pos = 0;
>> +	sf->count = 0;
> 
> --
> thanks,
> 
> Mimi