Re: [PATCH 06/31] KVM: x86: Introduce memory fault on invalid hypercalls reads/writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: KVM_CAP_HCALL_FAULT_EXIT can be toggled dynamically at runtime without locks
--

commit 2924fdad4ef4a10a3e4ce84aa7cf7e1887f8928d
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86: Introduce memory fault on invalid hypercalls reads/writes

This commit introduces the KVM_CAP_HCALL_FAULT_EXIT capability to check
memory read or written by hypercalls. When enabled, an inaccessible input
or output page causes an exit with KVM_EXIT_MEMORY_FAULT. This allows
userspace to perform arbitrary actions when a hypercall refers to an
invalid address.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1b8f24577dbf5..9206593878ee9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c

[ ... ]

> @@ -4210,6 +4211,10 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		mutex_unlock(&kvm->lock);
>  		break;
>  	}
> +	case KVM_CAP_HCALL_FAULT_EXIT:
> +		kvm->arch.hcall_fault_exit = cap->args[0];

[Severity: Medium]
Is it safe to toggle this capability dynamically without synchronization?

According to the KVM review guidelines, new state flags and capabilities that
modify behavior should default to immutable to prevent unanticipated state
machine transitions. Modifying this flag without acquiring the VM lock or
verifying if vCPUs have already been created allows userspace to dynamically
flip the capability while vCPUs are running.

Does this also introduce a data race with concurrent readers in the hypercall
handlers, since the capability flag is written without atomic annotations
or locks?

> +		r = 0;
> +		break;
>  	default:
>  		r = -EINVAL;
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918135030.171564-2-pbonzini@redhat.com?part=6