From: David Matlack <dmatlack@google•com>
To: Paolo Bonzini <pbonzini@redhat•com>
Cc: kvm@vger•kernel.org, David Hildenbrand <david@redhat•com>,
Paul Walmsley <paul.walmsley@sifive•com>,
David Matlack <dmatlack@google•com>,
linux-riscv@lists•infradead.org,
Claudio Imbrenda <imbrenda@linux•ibm.com>,
Janosch Frank <frankja@linux•ibm.com>,
Marc Zyngier <maz@kernel•org>,
Huacai Chen <chenhuacai@kernel•org>,
Zenghui Yu <yuzenghui@huawei•com>,
Palmer Dabbelt <palmer@dabbelt•com>,
Christian Borntraeger <borntraeger@linux•ibm.com>,
Albert Ou <aou@eecs•berkeley.edu>,
Suzuki K Poulose <suzuki.poulose@arm•com>,
Nicholas Piggin <npiggin@gmail•com>,
Bibo Mao <maobibo@loongson•cn>,
loongarch@lists•linux.dev, Atish Patra <atishp@atishpatra•org>,
kvmarm@lists•linux.dev, linux-arm-kernel@lists•infradead.org,
Sean Christopherson <seanjc@google•com>,
linux-mips@vger•kernel.org, Oliver Upton <oliver.upton@linux•dev>,
James Morse <james.morse@arm•com>,
kvm-riscv@lists•infradead.org, Anup Patel <anup@brainfault•org>,
Tianrui Zhao <zhaotianrui@loongson•cn>,
linuxppc-dev@lists•ozlabs.org
Subject: [PATCH v3 2/3] KVM: Ensure new code that references immediate_exit gets extra scrutiny
Date: Fri, 3 May 2024 11:17:33 -0700 [thread overview]
Message-ID: <20240503181734.1467938-3-dmatlack@google.com> (raw)
In-Reply-To: <20240503181734.1467938-1-dmatlack@google.com>
Ensure that any new KVM code that references immediate_exit gets extra
scrutiny by renaming it to immediate_exit__unsafe in kernel code.
All fields in struct kvm_run are subject to TOCTOU races since they are
mapped into userspace, which may be malicious or buggy. To protect KVM,
this commit introduces a new macro that appends __unsafe to field names
in struct kvm_run, hinting to developers and reviewers that accessing
this field must be done carefully.
Apply the new macro to immediate_exit, since userspace can make
immediate_exit inconsistent with vcpu->wants_to_run, i.e. accessing
immediate_exit directly could lead to unexpected bugs in the future.
Signed-off-by: David Matlack <dmatlack@google•com>
---
include/uapi/linux/kvm.h | 15 ++++++++++++++-
virt/kvm/kvm_main.c | 2 +-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 2190adbe3002..3611ad3b9c2a 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -192,11 +192,24 @@ struct kvm_xen_exit {
/* Flags that describe what fields in emulation_failure hold valid data. */
#define KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES (1ULL << 0)
+/*
+ * struct kvm_run can be modified by userspace at any time, so KVM must be
+ * careful to avoid TOCTOU bugs. In order to protect KVM, HINT_UNSAFE_IN_KVM()
+ * renames fields in struct kvm_run from <symbol> to <symbol>__unsafe when
+ * compiled into the kernel, ensuring that any use within KVM is obvious and
+ * gets extra scrutiny.
+ */
+#ifdef __KERNEL__
+#define HINT_UNSAFE_IN_KVM(_symbol) _symbol##__unsafe
+#else
+#define HINT_UNSAFE_IN_KVM(_symbol) _symbol
+#endif
+
/* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
struct kvm_run {
/* in */
__u8 request_interrupt_window;
- __u8 immediate_exit;
+ __u8 HINT_UNSAFE_IN_KVM(immediate_exit);
__u8 padding1[6];
/* out */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index bdea5b978f80..2b29851a90bd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4425,7 +4425,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
synchronize_rcu();
put_pid(oldpid);
}
- vcpu->wants_to_run = !READ_ONCE(vcpu->run->immediate_exit);
+ vcpu->wants_to_run = !READ_ONCE(vcpu->run->immediate_exit__unsafe);
r = kvm_arch_vcpu_ioctl_run(vcpu);
vcpu->wants_to_run = false;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
next prev parent reply other threads:[~2024-05-03 18:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 18:17 [PATCH v3 0/3] KVM: Set vcpu->preempted/ready iff scheduled out while running David Matlack
2024-05-03 18:17 ` [PATCH v3 1/3] KVM: Introduce vcpu->wants_to_run David Matlack
2024-05-03 18:17 ` David Matlack [this message]
2024-05-03 18:17 ` [PATCH v3 3/3] KVM: Mark a vCPU as preempted/ready iff it's scheduled out while running David Matlack
2024-06-18 21:41 ` [PATCH v3 0/3] KVM: Set vcpu->preempted/ready iff " Sean Christopherson
2024-07-01 17:51 ` David Matlack
2024-07-10 15:51 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503181734.1467938-3-dmatlack@google.com \
--to=dmatlack@google$(echo .)com \
--cc=anup@brainfault$(echo .)org \
--cc=aou@eecs$(echo .)berkeley.edu \
--cc=atishp@atishpatra$(echo .)org \
--cc=borntraeger@linux$(echo .)ibm.com \
--cc=chenhuacai@kernel$(echo .)org \
--cc=david@redhat$(echo .)com \
--cc=frankja@linux$(echo .)ibm.com \
--cc=imbrenda@linux$(echo .)ibm.com \
--cc=james.morse@arm$(echo .)com \
--cc=kvm-riscv@lists$(echo .)infradead.org \
--cc=kvm@vger$(echo .)kernel.org \
--cc=kvmarm@lists$(echo .)linux.dev \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-mips@vger$(echo .)kernel.org \
--cc=linux-riscv@lists$(echo .)infradead.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=loongarch@lists$(echo .)linux.dev \
--cc=maobibo@loongson$(echo .)cn \
--cc=maz@kernel$(echo .)org \
--cc=npiggin@gmail$(echo .)com \
--cc=oliver.upton@linux$(echo .)dev \
--cc=palmer@dabbelt$(echo .)com \
--cc=paul.walmsley@sifive$(echo .)com \
--cc=pbonzini@redhat$(echo .)com \
--cc=seanjc@google$(echo .)com \
--cc=suzuki.poulose@arm$(echo .)com \
--cc=yuzenghui@huawei$(echo .)com \
--cc=zhaotianrui@loongson$(echo .)cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox