Re: [PATCH v4] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Setting an invalid arch_compat bypasses capability checks and poisons the shared vcore state, but only marks the calling vCPU as insane, allowing sibling vCPUs to exploit the invalid state.
- [High] The patch breaks the KVM kernel build for BookE and non-Book3S PowerPC configurations by dereferencing an incomplete type `struct kvmppc_vcore`.
--

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

KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

This commit introduces a validation mechanism to detect and reject
unsupported arch_compat values during guest initialization on PowerPC
Book3S HV. It uses a sentinel value to mark mismatched compatibility
configurations and updates kvmppc_sanity_check() to prevent the vCPU
from running.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 61dbeea317f3b..f9380ef657500 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
[ ... ]
> @@ -446,7 +446,19 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
[ ... ]
>  		case PVR_ARCH_31_P11:
> +			/*
> +			 * Need to check this for ISA 3.1, as Power10 and
> +			 * Power11 share the same PCR. For any subsequent ISA
> +			 * versions, this will be taken care of by the guest vs
> +			 * host PCR comparison below.
> +			 */
> +			if (!cpu_has_feature(CPU_FTR_P11_PVR)) {
> +				arch_compat = PVR_ARCH_INVALID;
> +				goto out;
> +			}
>  			guest_pcr_bit = PCR_ARCH_31;
>  			break;
>  		default:
> @@ -469,6 +481,7 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
>  			return -EINVAL;
>  	}
>  
> +out:
>  	spin_lock(&vc->lock);
>  	vc->arch_compat = arch_compat;
>  	kvmhv_nestedv2_mark_dirty(vcpu, KVMPPC_GSID_LOGICAL_PVR);
> @@ -479,7 +492,7 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
>  	vc->pcr = (host_pcr_bit - guest_pcr_bit) | PCR_MASK;
>  	spin_unlock(&vc->lock);
>  
> -	return 0;
> +	return kvmppc_sanity_check(vcpu);

[Severity: High]
If we set arch_compat to PVR_ARCH_INVALID and take the goto out path, does
this bypass the guest_pcr_bit > host_pcr_bit check and persist the sentinel
value into the shared vc->arch_compat?

Since kvmppc_sanity_check() is only called for the current vcpu, could
sibling vCPUs sharing the same vcore bypass the sanity check and run with
this corrupted vcore state?

>  }
>  
>  static void kvmppc_dump_regs(struct kvm_vcpu *vcpu)
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 00302399fc37b..98de68379b18d 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -258,6 +258,10 @@ int kvmppc_sanity_check(struct kvm_vcpu *vcpu)
>  	if (!vcpu->arch.pvr)
>  		goto out;
>  
> +	if (vcpu->arch.vcore &&
> +		vcpu->arch.vcore->arch_compat == PVR_ARCH_INVALID)
> +		goto out;
> +

[Severity: High]
Does this break the build for BookE and non-Book3S configurations? 

kvmppc_sanity_check() is common code located in arch/powerpc/kvm/powerpc.c,
but struct kvmppc_vcore is only defined in asm/kvm_book3s.h.

If a BookE environment compiles this, won't dereferencing
vcpu->arch.vcore->arch_compat result in an incomplete type error?

>  	/* PAPR only works with book3s_64 */
>  	if ((vcpu->arch.cpu_type != KVM_CPU_3S_64) && vcpu->arch.papr_enabled)
>  		goto out;

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