|
Jiri Slaby |
c392bf |
From: "Dae R. Jeong" <threeearcat@gmail.com>
|
|
Jiri Slaby |
c392bf |
Date: Mon, 27 Mar 2023 21:01:53 +0900
|
|
Jiri Slaby |
c392bf |
Subject: [PATCH] vmci_host: fix a race condition in vmci_host_poll() causing
|
|
Jiri Slaby |
c392bf |
GPF
|
|
Jiri Slaby |
c392bf |
References: bsc#1012628
|
|
Jiri Slaby |
c392bf |
Patch-mainline: 6.3.2
|
|
Jiri Slaby |
c392bf |
Git-commit: ae13381da5ff0e8e084c0323c3cc0a945e43e9c7
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
[ Upstream commit ae13381da5ff0e8e084c0323c3cc0a945e43e9c7 ]
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
During fuzzing, a general protection fault is observed in
|
|
Jiri Slaby |
c392bf |
vmci_host_poll().
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
general protection fault, probably for non-canonical address 0xdffffc0000000019: 0000 [#1] PREEMPT SMP KASAN
|
|
Jiri Slaby |
c392bf |
KASAN: null-ptr-deref in range [0x00000000000000c8-0x00000000000000cf]
|
|
Jiri Slaby |
c392bf |
RIP: 0010:__lock_acquire+0xf3/0x5e00 kernel/locking/lockdep.c:4926
|
|
Jiri Slaby |
c392bf |
<- omitting registers ->
|
|
Jiri Slaby |
c392bf |
Call Trace:
|
|
Jiri Slaby |
c392bf |
<TASK>
|
|
Jiri Slaby |
c392bf |
lock_acquire+0x1a4/0x4a0 kernel/locking/lockdep.c:5672
|
|
Jiri Slaby |
c392bf |
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
|
|
Jiri Slaby |
c392bf |
_raw_spin_lock_irqsave+0xb3/0x100 kernel/locking/spinlock.c:162
|
|
Jiri Slaby |
c392bf |
add_wait_queue+0x3d/0x260 kernel/sched/wait.c:22
|
|
Jiri Slaby |
c392bf |
poll_wait include/linux/poll.h:49 [inline]
|
|
Jiri Slaby |
c392bf |
vmci_host_poll+0xf8/0x2b0 drivers/misc/vmw_vmci/vmci_host.c:174
|
|
Jiri Slaby |
c392bf |
vfs_poll include/linux/poll.h:88 [inline]
|
|
Jiri Slaby |
c392bf |
do_pollfd fs/select.c:873 [inline]
|
|
Jiri Slaby |
c392bf |
do_poll fs/select.c:921 [inline]
|
|
Jiri Slaby |
c392bf |
do_sys_poll+0xc7c/0x1aa0 fs/select.c:1015
|
|
Jiri Slaby |
c392bf |
__do_sys_ppoll fs/select.c:1121 [inline]
|
|
Jiri Slaby |
c392bf |
__se_sys_ppoll+0x2cc/0x330 fs/select.c:1101
|
|
Jiri Slaby |
c392bf |
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
|
|
Jiri Slaby |
c392bf |
do_syscall_64+0x4e/0xa0 arch/x86/entry/common.c:82
|
|
Jiri Slaby |
c392bf |
entry_SYSCALL_64_after_hwframe+0x46/0xb0
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
Example thread interleaving that causes the general protection fault
|
|
Jiri Slaby |
c392bf |
is as follows:
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
CPU1 (vmci_host_poll) CPU2 (vmci_host_do_init_context)
|
|
Jiri Slaby |
c392bf |
----- -----
|
|
Jiri Slaby |
c392bf |
// Read uninitialized context
|
|
Jiri Slaby |
c392bf |
context = vmci_host_dev->context;
|
|
Jiri Slaby |
c392bf |
// Initialize context
|
|
Jiri Slaby |
c392bf |
vmci_host_dev->context = vmci_ctx_create();
|
|
Jiri Slaby |
c392bf |
vmci_host_dev->ct_type = VMCIOBJ_CONTEXT;
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) {
|
|
Jiri Slaby |
c392bf |
// Dereferencing the wrong pointer
|
|
Jiri Slaby |
c392bf |
poll_wait(..., &context->host_context);
|
|
Jiri Slaby |
c392bf |
}
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
In this scenario, vmci_host_poll() reads vmci_host_dev->context first,
|
|
Jiri Slaby |
c392bf |
and then reads vmci_host_dev->ct_type to check that
|
|
Jiri Slaby |
c392bf |
vmci_host_dev->context is initialized. However, since these two reads
|
|
Jiri Slaby |
c392bf |
are not atomically executed, there is a chance of a race condition as
|
|
Jiri Slaby |
c392bf |
described above.
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
To fix this race condition, read vmci_host_dev->context after checking
|
|
Jiri Slaby |
c392bf |
the value of vmci_host_dev->ct_type so that vmci_host_poll() always
|
|
Jiri Slaby |
c392bf |
reads an initialized context.
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
Reported-by: Dae R. Jeong <threeearcat@gmail.com>
|
|
Jiri Slaby |
c392bf |
Fixes: 8bf503991f87 ("VMCI: host side driver implementation.")
|
|
Jiri Slaby |
c392bf |
Signed-off-by: Dae R. Jeong <threeearcat@gmail.com>
|
|
Jiri Slaby |
c392bf |
Link: https://lore.kernel.org/r/ZCGFsdBAU4cYww5l@dragonet
|
|
Jiri Slaby |
c392bf |
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Jiri Slaby |
c392bf |
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
Jiri Slaby |
c392bf |
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
|
|
Jiri Slaby |
c392bf |
---
|
|
Jiri Slaby |
c392bf |
drivers/misc/vmw_vmci/vmci_host.c | 8 +++++++-
|
|
Jiri Slaby |
c392bf |
1 file changed, 7 insertions(+), 1 deletion(-)
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
|
|
Jiri Slaby |
c392bf |
index 857b98514..abe79f6f 100644
|
|
Jiri Slaby |
c392bf |
--- a/drivers/misc/vmw_vmci/vmci_host.c
|
|
Jiri Slaby |
c392bf |
+++ b/drivers/misc/vmw_vmci/vmci_host.c
|
|
Jiri Slaby |
c392bf |
@@ -165,10 +165,16 @@ static int vmci_host_close(struct inode *inode, struct file *filp)
|
|
Jiri Slaby |
c392bf |
static __poll_t vmci_host_poll(struct file *filp, poll_table *wait)
|
|
Jiri Slaby |
c392bf |
{
|
|
Jiri Slaby |
c392bf |
struct vmci_host_dev *vmci_host_dev = filp->private_data;
|
|
Jiri Slaby |
c392bf |
- struct vmci_ctx *context = vmci_host_dev->context;
|
|
Jiri Slaby |
c392bf |
+ struct vmci_ctx *context;
|
|
Jiri Slaby |
c392bf |
__poll_t mask = 0;
|
|
Jiri Slaby |
c392bf |
|
|
Jiri Slaby |
c392bf |
if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) {
|
|
Jiri Slaby |
c392bf |
+ /*
|
|
Jiri Slaby |
c392bf |
+ * Read context only if ct_type == VMCIOBJ_CONTEXT to make
|
|
Jiri Slaby |
c392bf |
+ * sure that context is initialized
|
|
Jiri Slaby |
c392bf |
+ */
|
|
Jiri Slaby |
c392bf |
+ context = vmci_host_dev->context;
|
|
Jiri Slaby |
c392bf |
+
|
|
Jiri Slaby |
c392bf |
/* Check for VMCI calls to this VM context. */
|
|
Jiri Slaby |
c392bf |
if (wait)
|
|
Jiri Slaby |
c392bf |
poll_wait(filp, &context->host_context.wait_queue,
|
|
Jiri Slaby |
c392bf |
--
|
|
Jiri Slaby |
c392bf |
2.35.3
|
|
Jiri Slaby |
c392bf |
|