All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests
@ 2026-08-06 21:46 Sean Christopherson
  2026-08-06 22:08 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-08-06 21:46 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, Sean Christopherson, Paolo Bonzini
  Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
	linux-arm-kernel, kvmarm, kvm, linux-kernel

Use kvm_test_request() instead of kvm_check_request() when querying
KVM_REQ_VM_DEAD, i.e. don't clear KVM_REQ_VM_DEAD, as the entire purpose
of KVM_REQ_VM_DEAD is to prevent the vCPU from enterring the guest ever
again, even if userspace insists on redoing KVM_RUN.

Ensuring KVM_REQ_VM_DEAD is never cleared will allow relaxing KVM's rule
that ioctls can't be invoked on dead VMs, to only disallow ioctls if the
VM is bugged, i.e. if KVM hit a KVM_BUG_ON().

Opportunistically add compile-time assertions to guard against clearing
KVM_REQ_VM_DEAD through the standard APIs.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---

This was previously posted as part of a series to add an ioctl to allow
userspace to terminate a TDX VM.  I'm posting it separately because it fixes
a very real flaw, although the flaw is probably benign in practice?  I don't
want to find out though, i.e. I don't want to wait to get this merged.

This came back on my radar when fixing a bug in x86's pre-fault code, and
looking at that code made it pretty clear that clearing KVM_REQ_VM_DEAD could
make a bad situation worse.

 arch/arm64/kvm/arm.c     | 2 +-
 arch/x86/kvm/mmu/mmu.c   | 2 +-
 arch/x86/kvm/vmx/tdx.c   | 2 +-
 arch/x86/kvm/x86.c       | 2 +-
 include/linux/kvm_host.h | 9 +++++++--
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 50adfff75be8..a56ead423897 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1108,7 +1108,7 @@ static int kvm_vcpu_suspend(struct kvm_vcpu *vcpu)
 static int check_vcpu_requests(struct kvm_vcpu *vcpu)
 {
 	if (kvm_request_pending(vcpu)) {
-		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu))
+		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
 			return -EIO;
 
 		if (kvm_check_request(KVM_REQ_SLEEP, vcpu))
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c519e8e8d646..ec34caa06b6a 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5033,7 +5033,7 @@ static int kvm_tdp_page_prefault(struct kvm_vcpu *vcpu, gpa_t gpa,
 		if (signal_pending(current))
 			return -EINTR;
 
-		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu))
+		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
 			return -EIO;
 
 		cond_resched();
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b272c20586a7..6c842e9191a5 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1995,7 +1995,7 @@ static int tdx_handle_ept_violation(struct kvm_vcpu *vcpu)
 		if (kvm_vcpu_has_events(vcpu) || signal_pending(current))
 			break;
 
-		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu)) {
+		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu)) {
 			ret = -EIO;
 			break;
 		}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d94b59140c45..0af4b4a95c2d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8060,7 +8060,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	bool req_immediate_exit = false;
 
 	if (kvm_request_pending(vcpu)) {
-		if (kvm_check_request(KVM_REQ_VM_DEAD, vcpu)) {
+		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu)) {
 			r = -EIO;
 			goto out;
 		}
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6..cf7fe835c4ad 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2324,13 +2324,18 @@ static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
 	return test_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
 }
 
-static inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
+static __always_inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
 {
+	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
+
 	clear_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
 }
 
-static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
+static __always_inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
 {
+	/* Once a VM is dead, it needs to stay dead. */
+	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
+
 	if (kvm_test_request(req, vcpu)) {
 		kvm_clear_request(req, vcpu);
 

base-commit: a806d364ef288a6443a1337820ea8410a7ccc6b3
-- 
2.55.0.679.g6767b8d81c-goog



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests
  2026-08-06 21:46 [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests Sean Christopherson
@ 2026-08-06 22:08 ` sashiko-bot
  2026-08-07  8:20 ` Marc Zyngier
  2026-08-10  1:29 ` Huang, Kai
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-06 22:08 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, kvmarm, Oliver Upton, Marc Zyngier

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

Pre-existing issues:
- [High] Unhandled KVM_REQ_VM_DEAD across multiple architectures (s390, RISC-V, PowerPC, LoongArch) causes dead VMs to either improperly resume guest execution or get stuck in an infinite kernel loop.
--

--- Patch [1]: [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests ---
commit c8b0ec7047292dc9b84167da2e2a4d9abf600813
Author: Sean Christopherson <seanjc@google.com>

KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests

This commit modifies KVM_REQ_VM_DEAD checks to use kvm_test_request()
instead of kvm_check_request(), ensuring the flag is never cleared. It
also adds a build-time assertion to guard against clearing it via standard
APIs.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 03bfc92864b6e..cf7fe835c4ad2 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2324,13 +2324,18 @@ static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
>  	return test_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
>  }
>  
> -static inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
> +static __always_inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
>  {
> +	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
> +
>  	clear_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
>  }
>  
> -static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
> +static __always_inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
>  {
> +	/* Once a VM is dead, it needs to stay dead. */
> +	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
> +

[Severity: High]
This is a pre-existing issue, but while tightening the KVM_REQ_VM_DEAD API,
should this request be explicitly handled on other architectures?

Currently, s390, RISC-V, PowerPC, and LoongArch do not check for
KVM_REQ_VM_DEAD in their request handling loops. Because the flag is never
handled, kvm_request_pending() remains permanently true when a VM is marked
dead (e.g., via KVM_BUG_ON() in common pfncache or MMIO paths).

Could this cause the vCPU run loops to infinitely spin inside the kernel
without returning to userspace? For example:
- arch/riscv/kvm/vcpu.c:kvm_riscv_check_vcpu_requests()
- arch/powerpc/kvm/powerpc.c:kvm_arch_vcpu_ioctl_run() calling
  kvmppc_core_check_requests()
- arch/loongarch/kvm/vcpu.c:kvm_check_requests() inside kvm_pre_enter_guest()

Additionally, for s390:
- arch/s390/kvm/kvm-s390.c:kvm_s390_handle_requests() returns 0 when no
  other requests are handled. Can this cause the dead VM to improperly
  resume guest execution via sie64a() with a broken host state?

>  	if (kvm_test_request(req, vcpu)) {
>  		kvm_clear_request(req, vcpu);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806214618.82180-1-seanjc@google.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests
  2026-08-06 21:46 [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests Sean Christopherson
  2026-08-06 22:08 ` sashiko-bot
@ 2026-08-07  8:20 ` Marc Zyngier
  2026-08-10  1:29 ` Huang, Kai
  2 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2026-08-07  8:20 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Oliver Upton, Paolo Bonzini, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, linux-arm-kernel, kvmarm, kvm,
	linux-kernel

On Thu, 06 Aug 2026 22:46:18 +0100,
Sean Christopherson <seanjc@google.com> wrote:
> 
> Use kvm_test_request() instead of kvm_check_request() when querying
> KVM_REQ_VM_DEAD, i.e. don't clear KVM_REQ_VM_DEAD, as the entire purpose
> of KVM_REQ_VM_DEAD is to prevent the vCPU from enterring the guest ever
> again, even if userspace insists on redoing KVM_RUN.
> 
> Ensuring KVM_REQ_VM_DEAD is never cleared will allow relaxing KVM's rule
> that ioctls can't be invoked on dead VMs, to only disallow ioctls if the
> VM is bugged, i.e. if KVM hit a KVM_BUG_ON().
> 
> Opportunistically add compile-time assertions to guard against clearing
> KVM_REQ_VM_DEAD through the standard APIs.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests
  2026-08-06 21:46 [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests Sean Christopherson
  2026-08-06 22:08 ` sashiko-bot
  2026-08-07  8:20 ` Marc Zyngier
@ 2026-08-10  1:29 ` Huang, Kai
  2 siblings, 0 replies; 4+ messages in thread
From: Huang, Kai @ 2026-08-10  1:29 UTC (permalink / raw)
  To: maz@kernel.org, pbonzini@redhat.com, oupton@kernel.org,
	seanjc@google.com
  Cc: yuzenghui@huawei.com, suzuki.poulose@arm.com,
	kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	joey.gouly@arm.com, seiden@linux.ibm.com

On Thu, 2026-08-06 at 14:46 -0700, Sean Christopherson wrote:
> Use kvm_test_request() instead of kvm_check_request() when querying
> KVM_REQ_VM_DEAD, i.e. don't clear KVM_REQ_VM_DEAD, as the entire purpose
> of KVM_REQ_VM_DEAD is to prevent the vCPU from enterring the guest ever
> again, even if userspace insists on redoing KVM_RUN.
> 
> Ensuring KVM_REQ_VM_DEAD is never cleared will allow relaxing KVM's rule
> that ioctls can't be invoked on dead VMs, to only disallow ioctls if the
> VM is bugged, i.e. if KVM hit a KVM_BUG_ON().
> 
> Opportunistically add compile-time assertions to guard against clearing
> KVM_REQ_VM_DEAD through the standard APIs.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

FWIW, I build tested that both gcc and clang could build successfully (both -O2
and -Os).

[...]

> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2324,13 +2324,18 @@ static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
>  	return test_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
>  }
>  
> -static inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
> +static __always_inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
>  {
> +	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
> +
>  	clear_bit(req & KVM_REQUEST_MASK, (void *)&vcpu->requests);
>  }
>  
> -static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
> +static __always_inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
>  {
> +	/* Once a VM is dead, it needs to stay dead. */
> +	BUILD_BUG_ON(req == KVM_REQ_VM_DEAD);
> +
>  	if (kvm_test_request(req, vcpu)) {
>  		kvm_clear_request(req, vcpu);
>  

Nit: AFAICT the change to __always_inline is to avoid build failure.  Perhaps
explicitly mention this in changelog?

Btw, I also tried building the kernel after reverting __always_inline to plain
inline, but indeed got build error when using clang (clang failed with both -O2
and -Os, but gcc was fine for both, though).

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-10  1:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 21:46 [PATCH] KVM: Never clear KVM_REQ_VM_DEAD from a vCPU's requests Sean Christopherson
2026-08-06 22:08 ` sashiko-bot
2026-08-07  8:20 ` Marc Zyngier
2026-08-10  1:29 ` Huang, Kai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.