Re: [PATCH v4 3/3] KVM: PPC: Fix get_d_signext() 12-bit displacement for paired-single D-form

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kvm: ppc: out-of-bounds host memory access in magic page emulation
--

commit 7a374e4096a679cdf2566d00b45341748bfbac50
Author: Amit Machhiwal <amachhiw@linux.ibm.com>

KVM: PPC: Fix get_d_signext() 12-bit displacement for paired-single D-form

This commit fixes two compounding bugs in get_d_signext() related to
12-bit D-form displacements for paired-single instructions. It replaces
open-coded logic with sign_extend32 to correctly handle bit extraction
and two's-complement sign extension.

> diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
> index bc39c76c9d9fe..532f96293de0f 100644
> --- a/arch/powerpc/kvm/book3s_paired_singles.c
> +++ b/arch/powerpc/kvm/book3s_paired_singles.c
> @@ -479,12 +479,7 @@ static bool kvmppc_inst_is_paired_single(struct kvm_vcpu *vcpu, u32 inst)
>  
>  static int get_d_signext(u32 inst)
>  {
> -	int d = inst & 0x8ff;
> -
> -	if (d & 0x800)
> -		return -(d & 0x7ff);
> -
> -	return (d & 0x7ff);
> +	return sign_extend32(inst & 0xfff, 11);
>  }

[Severity: Critical]
By fixing the negative displacement calculation here, does this allow 
negative displacements (like -4 encoding to 0xffc) to reach the magic page 
emulation in kvmppc_st() and kvmppc_ld() without bounds checking?

For example, in arch/powerpc/kvm/powerpc.c:kvmppc_st():

	/* Magic page override */
	if (kvmppc_supports_magic_page(vcpu) && mp_pa &&
	    ((pte.raddr & KVM_PAM & PAGE_MASK) == mp_pa) &&
	    !(kvmppc_get_msr(vcpu) & MSR_PR)) {
		void *magic = vcpu->arch.shared;
		magic += pte.eaddr & 0xfff;
		memcpy(magic, ptr, size);
		return EMULATE_DONE;
	}

If a guest VM sets up a base register pointing to the magic page and 
executes an 8-byte memory access instruction (like psq_st or lfd) with an 
offset that translates to 0xffc, could this memcpy copy 8 bytes starting 
at offset 0xffc, overflowing the 4096-byte vcpu->arch.shared page?

A similar situation appears to exist in kvmppc_ld():

	/* Magic page override */
	if (kvmppc_supports_magic_page(vcpu) && mp_pa &&
	    ((pte.raddr & KVM_PAM & PAGE_MASK) == mp_pa) &&
	    !(kvmppc_get_msr(vcpu) & MSR_PR)) {
		void *magic = vcpu->arch.shared;
		magic += pte.eaddr & 0xfff;
		memcpy(ptr, magic, size);
		return EMULATE_DONE;
	}

Could this allow an 8-byte copy to read out-of-bounds host memory when 
the offset is 0xffc?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007182116.12479-1-amachhiw@linux.ibm.com?part=3