Re: [PATCH v3 2/7] ima: kexec: move ima log copy from kexec load to execute

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

On 1/12/24 09:06, Mimi Zohar wrote:
> 
>>>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>>>> index f989f5f1933b..bf758fd5062c 100644
>>>> --- a/kernel/kexec_file.c
>>>> +++ b/kernel/kexec_file.c
>>>> @@ -734,6 +734,14 @@ static int kexec_calculate_store_digests(struct kimage *image)
>>>>    		if (ksegment->kbuf == pi->purgatory_buf)
>>>>    			continue;
>>>>    
>>>> +		/*
>>>> +		 * Skip the segment if ima_segment_index is set and matches
>>>> +		 * the current index
>>>> +		 */
>>>> +		if (image->is_ima_segment_index_set &&
>>>> +		    i == image->ima_segment_index)
>>>> +			continue;
>>> With this change, the IMA segment is not included in the digest
>>> calculation, nor should it be included in the digest verification.
>>> However, I'm not seeing the matching code change in the digest
>>> verification.
>>>
>> Fair question.
>>
>> But I don't think anything else needs to be done here.
>>
>> The way kexec_calculate_store_digests() and verify_sha256_digest()
>> are implemented, it already skips verification of the segments if
>> the segment is not part of 'purgatory_sha_regions'.
>>
>> In kexec_calculate_store_digests(), my change is to 'continue' when the
>> segment is the IMA segment when the function is going through all the
>> segments in a for loop [1].
>>
>> Therefore in kexec_calculate_store_digests() -
>>    - crypto_shash_update() is not called for IMA segment [1].
>>    - sha_regions[j] is not updated with IMA segment  [1].
>>    - This 'sha_regions' variable later becomes 'purgatory_sha_regions'
>>      in kexec_calculate_store_digests  [1].
>>    - and verify_sha256_digest() only verifies 'purgatory_sha_regions'[2].
>>
>>    Since IMA segment is not part of the 'purgatory_sha_regions', it is
>>    not included in the verification as part of verify_sha256_digest().
>>
>>> Please make ignoring the IMA segment a separate patch.
>>>
>> Sure. Will do.
> Thank you for the explanation.  Please include in the patch description a
> statement about the "sha_regions" not including the IMA segment, so nothing is
> needed on the verify side.
Definitely.  Will do.