need help: patches to capture events between kexec load and execute

[Mimi Zohar]

On Wed, 2023-05-31 at 15:02 -0700, Tushar Sugandhi wrote:
> Hi Mimi,
> 
> On 5/31/23 04:39, Mimi Zohar wrote:
> > Hi Tushar,
> >
> > On Thu, 2023-05-25 at 10:21 -0700, Tushar Sugandhi wrote:
> >
> >> The issue of IMA measurements getting lost between kexec 'load' and 'execute' still exists.
> >> I verified it on the mainline kernel 6.4.rc3. See *Appendix A* for details.
> >>
> >> I went through Thiago's patches he wrote several years ago, and tried to develop a solution.
> >>
> >> I was facing some issues with physical to virtual address translation.
> >> One of my co-worker at Microsoft helped my re-write the logic which seems to be working.
> >> See the attached patch and *Appendix B* for details.
> >>
> >> The basic functionality is working. I need to polish the code, and handle error paths in a better way.
> >> But before doing that, I need your feedback on the fundamental approach.
> >> Since I am not a kexec expert, it’d be great if I could get help with the code review
> >> and also suggestions on scenarios to test to validate the patch thoroughly.
> >> Let me know if I should first post the patch as RFC on public forums for that.
> > Thanks, Tushar.   Measurements can and are currently being added to the
> > IMA measurement list between kexec load and execute, but are not being
> > carried across kexec.  These measurements also extend the TPM.  After
> > the soft reboot, without these additional measurements the IMA
> > measurement list cannot be verified against the TPM PCRs.
> Yup. IMA measurement list goes out of sync with TPM PCRs after the soft 
> reboot.
> So the measurement list cannot be verified against PCRs. That's the issue.
> Thanks for acknowledging it.
> > Your proposed patch, like Thiago's, saved the entire IMA measurement
> > list again.  Assuming the buffer size can't change between kexec load
> > and execute, as per the comment, why not just allocate the buffer on
> > kexec load and fill it on kexec exec?
> >
> > To simulate the existing behavior, fill the buffer with as many
> > complete measurement records as the buffer will hold.


> I was under the impression that I was doing the same. i.e. allocate at 
> 'load' and fill on 'execute'.
> But I realized that in my patch ima_dump_measurement_list() gets called 
> twice - once at 'load' and once at 'execute'.
> Is that what you meant by "my patch saved the entire IMA measurement 
> list again" ?

Exactly, there's no need for copying the measurement list twice.  The
first N number of measurements haven't changed.

> ima_dump_measurement_list() calls vmalloc, memset, and memcpy.
> So there is definitely some redundancy. Let me see how can I optimize it.

Please look at my suggestion.

> Attaching my original patch again for reference.

Patches should be posted inline.  Please don't do this.

-- 
thanks,

Mimi