Re: [PATCH v2 6/8] KVM: nSVM: Fetch missing DecodeAssist bytes for synthesized #NPF/#PF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing 32-bit linear address truncation when computing `rip + count` for instruction fetch.
--

--- Patch [6]: [PATCH v2 6/8] KVM: nSVM: Fetch missing DecodeAssist bytes for synthesized #NPF/#PF ---
commit 92b0b1d782b47781c3462fcdfb9a37ae5af201c8
Author: Tina Zhang <zhang_wei@open-hieco.net>

KVM: nSVM: Fetch missing DecodeAssist bytes for synthesized #NPF/#PF

This commit updates nested SVM to fetch missing DecodeAssist instruction
bytes directly from the L2 guest page tables when the emulator fetch cache
is incomplete. It honors CS limits and handles page faults, but skips tail
reads for SEV guests.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -75,6 +75,54 @@ static void nested_svm_clear_synthesized_insn_bytes(struct vcpu_svm *svm)

[ ... ]

> +static u8 nested_svm_fetch_insn_bytes(struct kvm_vcpu *vcpu, u8 *bytes,
> +				      u8 count, u8 max_bytes)
> +{
> +	struct kvm_mmu *mmu = vcpu->arch.walk_mmu;
> +	u64 access = PFERR_FETCH_MASK;
> +	gva_t rip = kvm_get_linear_rip(vcpu);
> +	struct x86_exception e;
> +
> +	if (to_svm(vcpu)->vmcb->save.cpl == 3)
> +		access |= PFERR_USER_MASK;
> +
> +	/*
> +	 * Hardware truncates the fetch at the CS limit.  CS.base and CS.limit
> +	 * checks do not apply in 64-bit mode.
> +	 */
> +	if (!is_64_bit_mode(vcpu)) {
> +		u32 eip = kvm_rip_read(vcpu);
> +		u32 limit = to_svm(vcpu)->vmcb->save.cs.limit;
> +
> +		if (eip > limit)
> +			return 0;
> +		max_bytes = min_t(u64, max_bytes, (u64)limit - eip + 1);
> +	}
> +
> +	count = min(count, max_bytes);
> +
> +	while (count < max_bytes) {
> +		unsigned int chunk = min_t(unsigned int, max_bytes - count,
> +				PAGE_SIZE - offset_in_page(rip + count));
> +		gpa_t gpa = mmu->gva_to_gpa(vcpu, mmu, rip + count, access, &e);

[Severity: Medium]
Does this properly handle linear address wrap-around for 32-bit guests?

For a 32-bit L2 guest, if CS.base + EIP is close to 0xffffffff, adding
count to rip can cause rip + count to exceed 4GB. Because rip is a
64-bit gva_t, the addition won't automatically truncate to 32 bits,
meaning the address will evaluate as greater than 4GB rather than wrapping
to 0.

Could this cause KVM to fetch instruction bytes from the wrong guest
physical address when the instruction fetch wraps around the 4GB boundary?

> +
> +		if (gpa == INVALID_GPA ||
> +		    kvm_vcpu_read_guest_page(vcpu, gpa_to_gfn(gpa),
> +					     bytes + count,
> +					     offset_in_page(gpa), chunk))
> +			break;
> +
> +		count += chunk;
> +	}
> +
> +	return count;
> +}

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