Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept

On 24-08-26 19:21, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `clear_page(svm->vmcb)` leaves the `control->event_inj` and `control->event_inj_err` fields intact across a SHUTDOWN reset, causing KVM to spuriously inject stale events into the newly reset guest and leading to an immediate crash or triple fault.
> --
> 
> --- Patch [1]: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept ---
> commit 887c839299b43bfbaf990edaf22e0335bb4c3119
> Author: Shivansh Dhiman <[email protected]>
> 
> KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
> 
> This commit updates KVM's handling of intercepted shutdowns for AMD SVM.
> Instead of clearing the entire VMCB, it now only clears the save area and a
> few undefined fields in the control area, aligning with updated APM
> specifications.
> 
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
> [ ... ]
>> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
> [ ... ]
>>  	if (!is_sev_es_guest(vcpu)) {
>> -		clear_page(svm->vmcb);
>> +		struct vmcb_control_area *control = &svm->vmcb->control;
>> +
>> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
>> +		control->int_ctl &= ~GENMASK(15, 0);
>> +		control->int_state = 0;
> 
> [Severity: High]
> Since clear_page() was replaced with targeted clearing, do we also need to
> clear control->event_inj and control->event_inj_err here?
> 
> According to the AMD APM, hardware does not clear the EVENTINJ field in the
> VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
> event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).
> 
> If the EVENTINJ field is left intact across this shutdown reset, will KVM
> execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?
> 
> Could this cause KVM to spuriously inject the old fault into the newly
> reset real-mode vCPU on its first instruction, leading to an immediate
> crash or an infinite triple-fault loop?
The latest APM says otherwise. The #VMEXIT sequence in APM ends with "clear
EVENTINJ field in VMCB". Hardware clears the field on every exit, so there
is nothing stale to carry across the shutdown reset and the scenario can't
arise.