linux-kernel - Re: [PATCH 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPU has been created

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aHB0M/aJwzuqYBG4@intel.com>
Date: Fri, 11 Jul 2025 10:17:23 +0800
From: Chao Gao <chao.gao@...el.com>
To: "Huang, Kai" <kai.huang@...el.com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "Edgecombe, Rick P"
	<rick.p.edgecombe@...el.com>, "bp@...en8.de" <bp@...en8.de>, "Li, Xiaoyao"
	<xiaoyao.li@...el.com>, "nikunj@....com" <nikunj@....com>,
	"thomas.lendacky@....com" <thomas.lendacky@....com>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"Yamahata, Isaku" <isaku.yamahata@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] KVM: x86: Reject KVM_SET_TSC_KHZ VM ioctl when vCPU
 has been created

>AFAICT the actual updating of kvm->arch.default_tsc_khz needs to be in the
>kvm->lock mutex too.

Yep.

>Please let me know if you found any issue?
>
>diff --git a/Documentation/virt/kvm/api.rst
>b/Documentation/virt/kvm/api.rst
>index 43ed57e048a8..86ea1e2b2737 100644
>--- a/Documentation/virt/kvm/api.rst
>+++ b/Documentation/virt/kvm/api.rst
>@@ -2006,7 +2006,7 @@ frequency is KHz.
>
> If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
> be used as a vm ioctl to set the initial tsc frequency of subsequently
>-created vCPUs.
>+created vCPUs.  It must be called before any vCPU is created.

		^^ remove one space here.

"must be" sounds like a mandatory action, but IIUC the vm ioctl is optional for
non-CC VMs. I'm not sure if this is just a problem of my interpretation.

To make the API documentation super clear, how about:

If the KVM_CAP_VM_TSC_CONTROL capability is advertised, this can also
be used as a vm ioctl to set the initial tsc frequency of vCPUs before
any vCPU is created. Attempting to call this vm ioctl after vCPU creation
will return an EINVAL error.

>
> 4.56 KVM_GET_TSC_KHZ
> --------------------
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 2806f7104295..4051c0cacb92 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -7199,9 +7199,12 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned
>int ioctl, unsigned long arg)
>                if (user_tsc_khz == 0)
>                        user_tsc_khz = tsc_khz;
>
>-               WRITE_ONCE(kvm->arch.default_tsc_khz, user_tsc_khz);
>-               r = 0;
>-
>+               mutex_lock(&kvm->lock);
>+               if (!kvm->created_vcpus) {
>+                       WRITE_ONCE(kvm->arch.default_tsc_khz,
>user_tsc_khz);
>+                       r = 0;
>+               }
>+               mutex_unlock(&kvm->lock);

LGTM.

>                goto out;
>        }
>        case KVM_GET_TSC_KHZ: {