Re: [PATCH v4 4/9] KVM: nSVM: Propagate hardware DecodeAssist bytes to VMCB12

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing live migration serialization for DecodeAssist instruction bytes and desynchronization of vmcb02_insn_bytes_fresh flag.
--

--- Patch [4]: [PATCH v4 4/9] KVM: nSVM: Propagate hardware DecodeAssist bytes to VMCB12 ---
commit 84578efc5f5e5753c950d333f88e1748a68f8d8c
Author: Tina Zhang <zhang_wei@open-hieco.net>

KVM: nSVM: Propagate hardware DecodeAssist bytes to VMCB12

DecodeAssists provides instruction bytes for nested page faults and
intercepted page faults caused by data accesses.  When the feature is
exposed to L1, copy fresh hardware-provided instruction bytes from VMCB02
to VMCB12 for these exits.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index d08c30e9a6f44..d34370b84d5b3 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c

[ ... ]

> +static void nested_svm_update_vmcb12_insn_bytes(struct kvm_vcpu *vcpu,
> +						struct vmcb *vmcb12,
> +						const struct vmcb *vmcb02)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	nested_svm_clear_insn_bytes(vmcb12);
> +
> +	if (!nested_svm_vmexit_supports_insn_bytes(vcpu, vmcb02))
> +		goto out;
> +
> +	if (svm->nested.vmcb02_insn_bytes_fresh)
> +		nested_svm_copy_insn_bytes(vmcb12, vmcb02);

[Severity: High]
If a live migration occurs while KVM has a pending hardware #PF VM-exit
(where exception_vmexit.pending = true and vmcb02_insn_bytes_fresh = true),
how does the destination hypervisor know to restore this state?

It appears svm_get_nested_state() and svm_set_nested_state() aren't updated
to serialize vmcb02_insn_bytes_fresh or the hardware insn_bytes. Could this
cause the destination hypervisor to inject a #PF into VMCB12 with an
insn_len of 0, effectively losing the instruction bytes?

Additionally, if userspace calls KVM_SET_VCPU_EVENTS to clear a pending
exception:

arch/x86/kvm/x86.c:kvm_vcpu_ioctl_x86_set_vcpu_events() {
    ...
    vcpu->arch.exception_vmexit.pending = false;
    ...
}

The vmcb02_insn_bytes_fresh flag is not cleared alongside it. Could a
subsequent emulator-generated #PF then incorrectly copy these stale
instruction bytes into VMCB12?

> +
> +out:
> +	svm->nested.vmcb02_insn_bytes_fresh = false;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787116250.git.zhang_wei@open-hieco.net?part=4