From: Yewon Choi <woni9911@gmail.com>
To: Bryan Tan <bryantan@vmware.com>, Vishnu Dasa <vdasa@vmware.com>,
VMware PV-Drivers Reviewers <pv-drivers@vmware.com>,
Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org
Cc: threeearcat@gmail.com
Subject: [PATCH] vmci_host: use smp_load_acquire/smp_store_release when accessing vmci_host_dev->ct_type
Date: Wed, 22 Nov 2023 21:20:08 +0900 [thread overview]
Message-ID: <20231122122005.GA4661@libra05> (raw)
In vmci_host.c, missing memory barrier between vmci_host_dev->ct_type
and vmci_host_dev->context may cause uninitialized data access.
One of possible execution flows is as follows:
CPU 1 (vmci_host_do_init_context)
=====
vmci_host_dev->context = vmci_ctx_create(...) // 1
vmci_host_dev->ct_type = VMCIOBJ_CONTEXT; // 2
CPU 2 (vmci_host_poll)
=====
if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) { // 3
context = vmci_host_dev->context; // 4
poll_wait(..., &context->host_context.wait_queue, ...);
While ct_type serves as a flag indicating that context is initialized,
there is no memory barrier which prevents reordering between
1,2 and 3, 4. So it is possible that 4 reads uninitialized
vmci_host_dev->context.
In this case, the null dereference occurs in poll_wait().
In order to prevent this kind of reordering, we change plain accesses
to ct_type into smp_load_acquire() and smp_store_release().
Signed-off-by: Yewon Choi <woni9911@gmail.com>
---
drivers/misc/vmw_vmci/vmci_host.c | 40 ++++++++++++++++++-------------
1 file changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index abe79f6fd2a7..e83b6e0fe55b 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -139,7 +139,7 @@ static int vmci_host_close(struct inode *inode, struct file *filp)
{
struct vmci_host_dev *vmci_host_dev = filp->private_data;
- if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) == VMCIOBJ_CONTEXT) {
vmci_ctx_destroy(vmci_host_dev->context);
vmci_host_dev->context = NULL;
@@ -168,7 +168,7 @@ static __poll_t vmci_host_poll(struct file *filp, poll_table *wait)
struct vmci_ctx *context;
__poll_t mask = 0;
- if (vmci_host_dev->ct_type == VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) == VMCIOBJ_CONTEXT) {
/*
* Read context only if ct_type == VMCIOBJ_CONTEXT to make
* sure that context is initialized
@@ -309,7 +309,7 @@ static int vmci_host_do_init_context(struct vmci_host_dev *vmci_host_dev,
mutex_lock(&vmci_host_dev->lock);
- if (vmci_host_dev->ct_type != VMCIOBJ_NOT_SET) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_NOT_SET) {
vmci_ioctl_err("received VMCI init on initialized handle\n");
retval = -EINVAL;
goto out;
@@ -346,7 +346,13 @@ static int vmci_host_do_init_context(struct vmci_host_dev *vmci_host_dev,
goto out;
}
- vmci_host_dev->ct_type = VMCIOBJ_CONTEXT;
+ /*
+ * Make sure that ct_type is written after
+ * vmci_host_dev->context is initialized.
+ *
+ * This pairs with smp_load_acquire() in vmci_host_XXX.
+ */
+ smp_store_release(&vmci_host_dev->ct_type, VMCIOBJ_CONTEXT);
atomic_inc(&vmci_host_active_users);
vmci_call_vsock_callback(true);
@@ -366,7 +372,7 @@ static int vmci_host_do_send_datagram(struct vmci_host_dev *vmci_host_dev,
struct vmci_datagram *dg = NULL;
u32 cid;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -422,7 +428,7 @@ static int vmci_host_do_receive_datagram(struct vmci_host_dev *vmci_host_dev,
int retval;
size_t size;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -453,7 +459,7 @@ static int vmci_host_do_alloc_queuepair(struct vmci_host_dev *vmci_host_dev,
int vmci_status;
int __user *retptr;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -522,7 +528,7 @@ static int vmci_host_do_queuepair_setva(struct vmci_host_dev *vmci_host_dev,
struct vmci_qp_set_va_info __user *info = uptr;
s32 result;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -570,7 +576,7 @@ static int vmci_host_do_queuepair_setpf(struct vmci_host_dev *vmci_host_dev,
return -EINVAL;
}
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -641,7 +647,7 @@ static int vmci_host_do_qp_detach(struct vmci_host_dev *vmci_host_dev,
struct vmci_qp_dtch_info __user *info = uptr;
s32 result;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -668,7 +674,7 @@ static int vmci_host_do_ctx_add_notify(struct vmci_host_dev *vmci_host_dev,
s32 result;
u32 cid;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -691,7 +697,7 @@ static int vmci_host_do_ctx_remove_notify(struct vmci_host_dev *vmci_host_dev,
u32 cid;
int result;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -715,7 +721,7 @@ static int vmci_host_do_ctx_get_cpt_state(struct vmci_host_dev *vmci_host_dev,
void *cpt_buf;
int retval;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -747,7 +753,7 @@ static int vmci_host_do_ctx_set_cpt_state(struct vmci_host_dev *vmci_host_dev,
void *cpt_buf;
int retval;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -785,7 +791,7 @@ static int vmci_host_do_set_notify(struct vmci_host_dev *vmci_host_dev,
{
struct vmci_set_notify_info notify_info;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -818,7 +824,7 @@ static int vmci_host_do_notify_resource(struct vmci_host_dev *vmci_host_dev,
return -EINVAL;
}
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
@@ -867,7 +873,7 @@ static int vmci_host_do_recv_notifications(struct vmci_host_dev *vmci_host_dev,
u32 cid;
int retval = 0;
- if (vmci_host_dev->ct_type != VMCIOBJ_CONTEXT) {
+ if (smp_load_acquire(&vmci_host_dev->ct_type) != VMCIOBJ_CONTEXT) {
vmci_ioctl_err("only valid for contexts\n");
return -EINVAL;
}
--
2.37.3
next reply other threads:[~2023-11-22 12:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-22 12:20 Yewon Choi [this message]
2023-11-22 14:34 ` [PATCH] vmci_host: use smp_load_acquire/smp_store_release when accessing vmci_host_dev->ct_type Greg Kroah-Hartman
2023-11-23 7:49 ` Yewon Choi
2023-11-23 8:44 ` Greg Kroah-Hartman
2023-11-23 10:06 ` Dae R. Jeong
2023-11-23 10:14 ` Greg Kroah-Hartman
2023-11-23 11:08 ` Dae R. Jeong
2023-11-23 12:21 ` Greg Kroah-Hartman
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=20231122122005.GA4661@libra05 \
--to=woni9911@gmail.com \
--cc=arnd@arndb.de \
--cc=bryantan@vmware.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pv-drivers@vmware.com \
--cc=threeearcat@gmail.com \
--cc=vdasa@vmware.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.