Re: [PATCH v5 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Reading the `cpu-version` device tree property using `of_get_property` without validating its length can result in an out-of-bounds read if the property is malformed.
- [Low] Endianness type signature mismatch for `cpu_version` parameter causes Sparse static analysis warnings.

Pre-existing issues:
- [High] An unprivileged host user can trigger a `WARN_ON_ONCE` in `kvmppc_get_compat_caps()`, leading to a host denial of service.
--

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

KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV

This commit extends host CPU compatibility capability reporting to support
nested virtualization on PowerNV systems. For nested API v1, it derives the
compatibility capabilities from the L1 guest by reading the cpu-version
property from the device tree and mapping it to the corresponding KVM
capability bitmap.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8799124b80960..70dcd370fd9ac 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c

[ ... ]

> @@ -6510,20 +6510,58 @@ static bool kvmppc_hash_v3_possible(void)
>  	return true;
>  }
>  
> +static int kvmppc_map_compat_capabilities(const __be32 cpu_version,
> +				      unsigned long *capabilities)

[Severity: Low]
Is cpu_version actually host-endian here? The parameter is declared as
const __be32, but kvmppc_get_compat_caps() passes a host-endian u32 value
to it. Could this cause Sparse static analysis tools to warn about a
restricted __be32 degrading to an integer?

> +{
> +	switch (cpu_version) {
> +	case PVR_ARCH_31_P11:

[ ... ]

>  static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
>  {
> +	struct device_node *np;
>  	unsigned long capabilities = 0;
> +	const __be32 *prop = NULL;
>  	long rc = -EINVAL;
> +	u32 cpu_version;
>  
>  	if (kvmhv_on_pseries()) {
>  		if (kvmhv_is_nestedv2()) {
>  			WARN_ON_ONCE(!nested_capabilities);

[Severity: High]
This is a pre-existing issue, but can this assertion be triggered by
unprivileged userspace? 

If the KVM_PPC_GET_COMPAT_CAPS ioctl is called when nested_capabilities is 0
(for example, if the host hypervisor initializes nestedv2 but does not return
the capability flags), would this trigger a WARN_ON_ONCE? 

According to KVM guidelines, adversary-reachable WARN_ON assertions can be
host-side denial of service vectors if panic_on_warn is enabled.

>  			capabilities = nested_capabilities;
>  			rc = 0;
> +		} else {
> +			for_each_node_by_type(np, "cpu") {
> +				prop = of_get_property(np, "cpu-version", NULL);
> +				if (prop) {
> +					cpu_version = be32_to_cpup(prop);

[Severity: Low]
Could this read out of bounds if the cpu-version device tree property is
malformed and less than 4 bytes long?

The pointer returned by of_get_property() in kvmppc_get_compat_caps() is
dereferenced by be32_to_cpup() without checking the length. Would using
of_property_read_u32() natively validate the length and securely handle
the endianness conversion?

> +					of_node_put(np);
> +					break;
> +				}
> +			}

[ ... ]

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