Re: [PATCH v8 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl

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

New issues:
- [Medium] The documentation inaccurately describes how the extensible struct versioning handles trailing fields and truncation.

Pre-existing issues:
- [High] A userspace-reachable WARN_ON_ONCE in kvmppc_get_compat_caps allows unprivileged users to panic the host if panic_on_warn is set.
--

--- Patch [4]: [PATCH v8 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl ---
commit 991bf5faa645caad474f6369b275de7617351fd0
Author: Amit Machhiwal <[email protected]>

KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl

Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
documentation. The ioctl exposes host processor compatibility modes supported
for nested KVM guests on PowerPC systems. The documentation covers error
code descriptions including E2BIG for forward compatibility.

> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index e3003a241d5b0..e656d117cd0b1 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6566,6 +6566,95 @@ KVM_S390_KEYOP_SSKE
>    Sets the storage key for the guest address ``guest_addr`` to the key
>    specified in ``key``, returning the previous value in ``key``.
>  
> +4.145 KVM_PPC_GET_COMPAT_CAPS
> +-----------------------------

[Severity: High]
This is a pre-existing issue, but does this ioctl handler allow unprivileged
users to panic the host?

Looking at kvmppc_get_compat_caps() in arch/powerpc/kvm/book3s_hv.c:

arch/powerpc/kvm/book3s_hv.c:kvmppc_get_compat_caps() {
	if (kvmhv_on_pseries()) {
		if (kvmhv_is_nestedv2()) {
			WARN_ON_ONCE(!nested_capabilities);
			capabilities = nested_capabilities;
			[ ... ]
}

An unprivileged user with access to /dev/kvm can open a VM file descriptor
and call the KVM_PPC_GET_COMPAT_CAPS ioctl. If the system is a nested guest
(kvmhv_is_nestedv2() is true) but the parent hypervisor advertised no
capabilities (nested_capabilities is 0), the ioctl handler directly triggers
a WARN_ON_ONCE(!nested_capabilities).

This can occur naturally if plpar_guest_set_capabilities(0, 0) succeeds
during kvmhv_nested_init(). This directly violates the KVM subsystem
guidelines which explicitly forbid userspace-reachable WARN_ON assertions,
as it can cause a host kernel panic (Denial of Service) if panic_on_warn is
set.

[ ... ]

> +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
> +to support extensible versioning across three cases:
> +
> +- If ``size`` is smaller than the kernel's struct size (old userspace,
> +  new kernel), the kernel zero-pads the unknown trailing fields before
> +  returning, and writes back ``size`` unchanged so userspace knows how
> +  many bytes were filled.

[Severity: Medium]
Does this documentation accurately reflect the UAPI contract for struct padding
and truncation on the return path?

The documentation incorrectly states that if the userspace struct is smaller,
the kernel "zero-pads the unknown trailing fields before returning". This
misleadingly implies it pads the userspace buffer (which would be a buffer
overflow). In reality, the kernel strictly truncates trailing fields on output
to userspace using copy_struct_to_user().

Additionally, it fails to document that when userspace provides a larger
struct with zeroed trailing bytes, the kernel actively mutates the userspace
'size' field on success (via host_caps.size = min_t(...)) to reflect the
kernel's smaller struct size.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4