Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] KVM_CAP_PPC_COMPAT_CAPS capability is advertised but the ioctl backend is completely unimplemented, causing unconditional failure.
- [High] The KVM_PPC_GET_COMPAT_CAPS ioctl overwrites the userspace buffer's size field with a value larger than the allocated buffer, creating an out-of-bounds read risk in userspace.
- [Medium] The manual validation block explicitly rejecting `usize > sizeof(host_caps)` defeats the standard forward-compatibility guarantee of `copy_struct_from_user()`.
--

--- Patch [1]: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl ---
commit 8c51e17a26cbffd727576c50f93a34eea2340547
Author: Amit Machhiwal <amachhiw@linux.ibm.com>

KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl

Introduce a new capability and ioctl to expose CPU compatibility modes
supported by the host processor for nested guests.

> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 00302399fc37b..ed0cd7b83b63f 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
[ ... ]
> @@ -2463,6 +2470,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
[ ... ]
> +		/*
> +		 * New userspace with a larger struct called an older kernel.
> +		 * Write back ksize in host_caps.size so userspace knows which
> +		 * older struct to retry with, then fail with -E2BIG.
> +		 */
> +		if (usize > sizeof(host_caps)) {
> +			host_caps.size = sizeof(host_caps);
> +			r = -EFAULT;
> +			if (put_user(host_caps.size, (__u64 __user *)argp))
> +				goto out;
> +			r = -E2BIG;
> +			goto out;
> +		}

[Severity: Medium]
Does this manual size check defeat the standard forward-compatibility
guarantee of copy_struct_from_user()?

Normally, copy_struct_from_user() verifies that any trailing bytes provided
by a newer userspace are zero. By short-circuiting here and unconditionally
returning -E2BIG, aren't we rejecting perfectly valid, zero-initialized
structs and deviating from standard kernel extensible UAPI patterns (like
clone3 or openat2)?

[ ... ]
> +		r = -ENOTTY;
> +		if (!kvm->arch.kvm_ops->get_compat_caps)
> +			goto out;
> +
> +		r = kvm->arch.kvm_ops->get_compat_caps(&host_caps);
> +		if (r)
> +			goto out;

[Severity: High]
Will this ioctl always fail with -ENOTTY?

The capability KVM_CAP_PPC_COMPAT_CAPS is advertised as supported to
userspace (returning 1 on pseries hosts in kvm_vm_ioctl_check_extension()),
but the get_compat_caps backend function pointer is never assigned anywhere
in this patch.

If userspace attempts to use this ioctl based on the capability check,
won't it unconditionally hit this missing backend check and fail, causing
regressions in userspace startup flows?

> +
> +		host_caps.size = sizeof(host_caps);
> +		r = copy_struct_to_user(argp, usize, &host_caps,
> +					sizeof(host_caps), NULL);
> +		break;
> +	}

[Severity: High]
Is it safe to overwrite the userspace size field with the kernel's internal
struct size?

In a forward-compatibility scenario where an older userspace passes a
smaller struct to a newer kernel, usize will be smaller than
sizeof(host_caps). By setting host_caps.size = sizeof(host_caps) and
copying it back, the userspace struct will receive a larger size value than
it actually allocated.

If userspace trusts this size field for bounds checking or iteration, could
this regression lead to an out-of-bounds memory read in userspace? Shouldn't
the kernel preserve the originally requested usize?

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