kernel/cpu: add safety comments #584
Conversation
stac and clac instructions don't break memory safety. Signed-off-by: Thomas Leroy <[email protected]>
Writing to some MSRs can break memory safety. Therefore, write_msr() should be unsafe. Signed-off-by: Thomas Leroy <[email protected]>
Missing safety comments were missing in GDT code. Adding a couple of checks to validate the safety requirements. Signed-off-by: Thomas Leroy <[email protected]>
Both of these GHCB operations can cause physical memory to be remapped (one may cause the GHCB data to appear in a different place, and one may cause memory to be remapped with a different encryption attribute). Because both of these operations can cause mapped memory to change contents, they are technically unsafe. |
| // It's safe to do so as long as a global GDT is in use and still | ||
| // allocated, which is always our case. |
There was a problem hiding this comment.
Actually, it is not necessarily the case that a global GDT is in use here. However, the lifetime of the GDT doesn't matter, because once the task register is loaded, only the TSS needs to remain live. For that reason, either this whole function should be unsafe (because the compiler cannot prove that the lifetime of the tss parameter cannot be proven) or the tss parameter should be declared as &'static. A quick experiment suggests that the latter approach is workable as long as we modify PerCpu::load() and PerCpu::load_isst() to take &'static self. Once that is done, this comment should be updated to indicate that the assembly here is safe because tss has a static lifetime.
| @@ -577,7 +577,9 @@ mod tests { | |||
| fn test_wrmsr_tsc_aux() { | |||
| if is_qemu_test_env() && is_test_platform_type(SvsmPlatformType::Snp) { | |||
| let test_val = 0x1234; | |||
| verify_ghcb_gets_altered(|| write_msr(MSR_TSC_AUX, test_val)); | |||
| verify_ghcb_gets_altered(|| | |||
| // SAFETY: writing to TSC MSR doesn't break memory safety. | |||
There was a problem hiding this comment.
Should say TSC_AUX MSR.
Safety comments for:
stacandclacinstructionswrite_msr()should be unsafe since it can break memory safety. This part is still missing two comments here and here in the GHCB MSR protocol where I have doubt if the MSR request is subject to breaking memory safety or not, specificallySNP_REG_GHCB_GPA_REQandSNP_STATE_CHANGE_REQGHCB requests. My guess is a garbage request parameter can break the platform, but does it still count as memory safety?