On Sat, 2022-04-02 at 01:08 +0000, Sean Christopherson wrote:
> Unwind the RIP advancement done by svm_queue_exception() when injecting
> an INT3 ultimately "fails" due to the CPU encountering a VM-Exit while
> vectoring the injected event, even if the exception reported by the CPU
> isn't the same event that was injected. If vectoring INT3 encounters an
> exception, e.g. #NP, and vectoring the #NP encounters an intercepted
> exception, e.g. #PF when KVM is using shadow paging, then the #NP will
> be reported as the event that was in-progress.
>
> Note, this is still imperfect, as it will get a false positive if the
> INT3 is cleanly injected, no VM-Exit occurs before the IRET from the INT3
> handler in the guest, the instruction following the INT3 generates an
> exception (directly or indirectly), _and_ vectoring that exception
> encounters an exception that is intercepted by KVM. The false positives
> could theoretically be solved by further analyzing the vectoring event,
> e.g. by comparing the error code against the expected error code were an
> exception to occur when vectoring the original injected exception, but
> SVM without NRIPS is a complete disaster, trying to make it 100% correct
> is a waste of time.
Makes sense.
Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
Maxim Levitsky
>
> Fixes: 66b7138f9136 ("KVM: SVM: Emulate nRIP feature when reinjecting INT3")
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/svm/svm.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 2c86bd9176c6..30cef3b10838 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3699,6 +3699,18 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
> vector = exitintinfo & SVM_EXITINTINFO_VEC_MASK;
> type = exitintinfo & SVM_EXITINTINFO_TYPE_MASK;
>
> + /*
> + * If NextRIP isn't enabled, KVM must manually advance RIP prior to
> + * injecting the soft exception/interrupt. That advancement needs to
> + * be unwound if vectoring didn't complete. Note, the _new_ event may
> + * not be the injected event, e.g. if KVM injected an INTn, the INTn
> + * hit a #NP in the guest, and the #NP encountered a #PF, the #NP will
> + * be the reported vectored event, but RIP still needs to be unwound.
> + */
> + if (int3_injected && type == SVM_EXITINTINFO_TYPE_EXEPT &&
> + kvm_is_linear_rip(vcpu, svm->int3_rip))
> + kvm_rip_write(vcpu, kvm_rip_read(vcpu) - int3_injected);
> +
> switch (type) {
> case SVM_EXITINTINFO_TYPE_NMI:
> vcpu->arch.nmi_injected = true;
> @@ -3715,13 +3727,9 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
> * but re-execute the instruction instead. Rewind RIP first
> * if we emulated INT3 before.
> */
> - if (kvm_exception_is_soft(vector)) {
> - if (vector == BP_VECTOR && int3_injected &&
> - kvm_is_linear_rip(vcpu, svm->int3_rip))
> - kvm_rip_write(vcpu,
> - kvm_rip_read(vcpu) - int3_injected);
> + if (kvm_exception_is_soft(vector))
> break;
> - }
> +
> if (exitintinfo & SVM_EXITINTINFO_VALID_ERR) {
> u32 err = svm->vmcb->control.exit_int_info_err;
> kvm_requeue_exception_e(vcpu, vector, err);
