clear-pkgs-linux-iot-lts2018/0766-kernel-VHM-Fix-race-co...

587 lines
17 KiB
Diff

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Zhao Yakui <yakui.zhao@intel.com>
Date: Thu, 29 Nov 2018 13:58:15 +0800
Subject: [PATCH] kernel/VHM: Fix race condition in ioreq_client by using
refcnt and idr mechanism
Currently the ioreq_client is managed by using static array. it is possible that the
ioreq_client is still accessed while it is released in another thread. In such case
it will have the race-condition.
In order to avoid the race-condition, the refcnt is used to indicate whether it can
be released. And the IDR(alloc/find/replace) is used to manage the allocation/lookup/
destroy of ioreq_client(spin_lock_bh is used).
MAX_CLIENT is increased from 64 to 1024 after ioreq_client is managed by IDR.
V2: idr_remove is used directly instead of two steps(idr_replace and idr_remove).
Tracked-On: PKT-1592
Tracked-On: https://github.com/projectacrn/acrn-hypervisor/issues/1957
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Reviewed-by: Eddie Dong <eddie.dong@intel.com>
Tested-by: Yin, FengWei <fengwei.yin@intel.com>
---
drivers/char/vhm/vhm_dev.c | 1 +
drivers/vhm/vhm_ioreq.c | 192 ++++++++++++++++++++---------
include/linux/vhm/acrn_vhm_ioreq.h | 1 +
3 files changed, 136 insertions(+), 58 deletions(-)
diff --git a/drivers/char/vhm/vhm_dev.c b/drivers/char/vhm/vhm_dev.c
index 59dd49b6c84c..e6aa6f5d5aa3 100644
--- a/drivers/char/vhm/vhm_dev.c
+++ b/drivers/char/vhm/vhm_dev.c
@@ -815,6 +815,7 @@ static int __init vhm_init(void)
return -EINVAL;
}
+ acrn_ioreq_driver_init();
pr_info("vhm: Virtio & Hypervisor service module initialized\n");
return 0;
}
diff --git a/drivers/vhm/vhm_ioreq.c b/drivers/vhm/vhm_ioreq.c
index b0ca41a402ea..9d77bc3be95c 100644
--- a/drivers/vhm/vhm_ioreq.c
+++ b/drivers/vhm/vhm_ioreq.c
@@ -65,6 +65,10 @@
#include <linux/vhm/acrn_vhm_ioreq.h>
#include <linux/vhm/vhm_vm_mngt.h>
#include <linux/vhm/vhm_hypercall.h>
+#include <linux/idr.h>
+
+static DEFINE_SPINLOCK(client_lock);
+static struct idr idr_client;
struct ioreq_range {
struct list_head list;
@@ -127,12 +131,10 @@ struct ioreq_client {
int pci_bus;
int pci_dev;
int pci_func;
+ atomic_t refcnt;
};
-#define MAX_CLIENT 64
-static struct ioreq_client *clients[MAX_CLIENT];
-static DECLARE_BITMAP(client_bitmap, MAX_CLIENT);
-
+#define MAX_CLIENT 1024
static void acrn_ioreq_notify_client(struct ioreq_client *client);
static inline bool is_range_type(uint32_t type)
@@ -151,30 +153,48 @@ static inline bool has_pending_request(struct ioreq_client *client)
static int alloc_client(void)
{
struct ioreq_client *client;
- int i;
-
- i = find_first_zero_bit(client_bitmap, MAX_CLIENT);
- if (i >= MAX_CLIENT)
- return -ENOMEM;
- set_bit(i, client_bitmap);
+ int ret;
client = kzalloc(sizeof(struct ioreq_client), GFP_KERNEL);
if (!client)
return -ENOMEM;
- client->id = i;
+ atomic_set(&client->refcnt, 1);
+
+ spin_lock_bh(&client_lock);
+ ret = idr_alloc_cyclic(&idr_client, client, 1, MAX_CLIENT, GFP_NOWAIT);
+ spin_unlock_bh(&client_lock);
+
+ if (ret < 0) {
+ kfree(client);
+ return -EINVAL;
+ }
+
+ client->id = ret;
set_bit(IOREQ_CLIENT_EXIT, &client->flags);
- clients[i] = client;
- return i;
+ return ret;
}
-static void free_client(int i)
+static struct ioreq_client *acrn_ioreq_get_client(int client_id)
{
- if (i < MAX_CLIENT && i >= 0) {
- if (test_and_clear_bit(i, client_bitmap)) {
- kfree(clients[i]);
- clients[i] = NULL;
- }
+ struct ioreq_client *obj;
+
+ spin_lock_bh(&client_lock);
+ obj = idr_find(&idr_client, client_id);
+ if (obj)
+ atomic_inc(&obj->refcnt);
+ spin_unlock_bh(&client_lock);
+
+ return obj;
+}
+
+
+static void acrn_ioreq_put_client(struct ioreq_client *client)
+{
+ if (atomic_dec_and_test(&client->refcnt)) {
+ /* The client should be released when refcnt = 0 */
+ /* TBD: Do we need to free the other resources? */
+ kfree(client);
}
}
@@ -209,7 +229,12 @@ int acrn_ioreq_create_client(unsigned long vmid, ioreq_handler_t handler,
return -EINVAL;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
+ if (unlikely(client == NULL)) {
+ pr_err("failed to get the client.\n");
+ put_vm(vm);
+ return -EINVAL;
+ }
if (handler) {
client->handler = handler;
@@ -223,6 +248,7 @@ int acrn_ioreq_create_client(unsigned long vmid, ioreq_handler_t handler,
INIT_LIST_HEAD(&client->range_list);
init_waitqueue_head(&client->wq);
+ /* When it is added to ioreq_client_list, the refcnt is increased */
spin_lock_irqsave(&vm->ioreq_client_lock, flags);
list_add(&client->list, &vm->ioreq_client_list);
spin_unlock_irqrestore(&vm->ioreq_client_lock, flags);
@@ -271,10 +297,14 @@ void acrn_ioreq_clear_request(struct vhm_vm *vm)
/* Clear all ioreqs belong to DM. */
if (vm->ioreq_fallback_client > 0) {
- client = clients[vm->ioreq_fallback_client];
+ client = acrn_ioreq_get_client(vm->ioreq_fallback_client);
+ if (!client)
+ return;
+
while ((bit = find_next_bit(client->ioreqs_map,
0, VHM_REQUEST_MAX)) == VHM_REQUEST_MAX)
acrn_ioreq_complete_request(client->id, bit, NULL);
+ acrn_ioreq_put_client(client);
}
}
@@ -282,6 +312,7 @@ int acrn_ioreq_create_fallback_client(unsigned long vmid, char *name)
{
struct vhm_vm *vm;
int client_id;
+ struct ioreq_client *client;
vm = find_get_vm(vmid);
if (unlikely(vm == NULL)) {
@@ -304,14 +335,23 @@ int acrn_ioreq_create_fallback_client(unsigned long vmid, char *name)
return -EINVAL;
}
- clients[client_id]->fallback = true;
+ client = acrn_ioreq_get_client(client_id);
+ if (unlikely(client == NULL)) {
+ pr_err("failed to get the client.\n");
+ put_vm(vm);
+ return -EINVAL;
+ }
+
+ client->fallback = true;
vm->ioreq_fallback_client = client_id;
+ acrn_ioreq_put_client(client);
put_vm(vm);
return client_id;
}
+/* When one client is removed from VM, the refcnt is decreased */
static void acrn_ioreq_destroy_client_pervm(struct ioreq_client *client,
struct vhm_vm *vm)
{
@@ -340,7 +380,7 @@ static void acrn_ioreq_destroy_client_pervm(struct ioreq_client *client,
if (client->id == vm->ioreq_fallback_client)
vm->ioreq_fallback_client = -1;
- free_client(client->id);
+ acrn_ioreq_put_client(client);
}
void acrn_ioreq_destroy_client(int client_id)
@@ -352,11 +392,14 @@ void acrn_ioreq_destroy_client(int client_id)
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return;
}
- client = clients[client_id];
- if (!client) {
- pr_err("vhm-ioreq: no client for id %d\n", client_id);
+
+ spin_lock_bh(&client_lock);
+ client = idr_remove(&idr_client, client_id);
+ spin_unlock_bh(&client_lock);
+
+ /* When the client_id is already released, just keep silience can returnd */
+ if (!client)
return;
- }
might_sleep();
@@ -364,10 +407,12 @@ void acrn_ioreq_destroy_client(int client_id)
if (unlikely(vm == NULL)) {
pr_err("vhm-ioreq: failed to find vm from vmid %ld\n",
client->vmid);
+ acrn_ioreq_put_client(client);
return;
}
acrn_ioreq_destroy_client_pervm(client, vm);
+ acrn_ioreq_put_client(client);
put_vm(vm);
}
@@ -403,14 +448,14 @@ int acrn_ioreq_add_iorange(int client_id, uint32_t type,
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
}
- client = clients[client_id];
- if (!client) {
- pr_err("vhm-ioreq: no client for id %d\n", client_id);
+ if (end < start) {
+ pr_err("vhm-ioreq: end < start\n");
return -EFAULT;
}
- if (end < start) {
- pr_err("vhm-ioreq: end < start\n");
+ client = acrn_ioreq_get_client(client_id);
+ if (!client) {
+ pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
}
@@ -419,6 +464,7 @@ int acrn_ioreq_add_iorange(int client_id, uint32_t type,
range = kzalloc(sizeof(struct ioreq_range), GFP_KERNEL);
if (!range) {
pr_err("vhm-ioreq: failed to alloc ioreq range\n");
+ acrn_ioreq_put_client(client);
return -ENOMEM;
}
range->type = type;
@@ -428,6 +474,7 @@ int acrn_ioreq_add_iorange(int client_id, uint32_t type,
spin_lock_irqsave(&client->range_lock, flags);
list_add(&range->list, &client->range_list);
spin_unlock_irqrestore(&client->range_lock, flags);
+ acrn_ioreq_put_client(client);
return 0;
}
@@ -445,14 +492,14 @@ int acrn_ioreq_del_iorange(int client_id, uint32_t type,
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
}
- client = clients[client_id];
- if (!client) {
- pr_err("vhm-ioreq: no client for id %d\n", client_id);
+ if (end < start) {
+ pr_err("vhm-ioreq: end < start\n");
return -EFAULT;
}
- if (end < start) {
- pr_err("vhm-ioreq: end < start\n");
+ client = acrn_ioreq_get_client(client_id);
+ if (!client) {
+ pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
}
@@ -477,6 +524,7 @@ int acrn_ioreq_del_iorange(int client_id, uint32_t type,
}
}
spin_unlock_irqrestore(&client->range_lock, flags);
+ acrn_ioreq_put_client(client);
return 0;
}
@@ -499,7 +547,7 @@ struct vhm_request *acrn_ioreq_get_reqbuf(int client_id)
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return NULL;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
if (!client) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return NULL;
@@ -508,6 +556,7 @@ struct vhm_request *acrn_ioreq_get_reqbuf(int client_id)
if (unlikely(vm == NULL)) {
pr_err("vhm-ioreq: failed to find vm from vmid %ld\n",
client->vmid);
+ acrn_ioreq_put_client(client);
return NULL;
}
@@ -516,6 +565,7 @@ struct vhm_request *acrn_ioreq_get_reqbuf(int client_id)
"for vmid %ld\n", client->vmid);
}
put_vm(vm);
+ acrn_ioreq_put_client(client);
return (struct vhm_request *)vm->req_buf;
}
EXPORT_SYMBOL_GPL(acrn_ioreq_get_reqbuf);
@@ -525,8 +575,12 @@ static int ioreq_client_thread(void *data)
struct ioreq_client *client;
int ret, client_id = (unsigned long)data;
+ client = acrn_ioreq_get_client(client_id);
+
+ if (!client)
+ return 0;
+
while (1) {
- client = clients[client_id];
if (is_destroying(client)) {
pr_info("vhm-ioreq: client destroying->stop thread\n");
break;
@@ -550,7 +604,7 @@ static int ioreq_client_thread(void *data)
}
set_bit(IOREQ_CLIENT_EXIT, &client->flags);
-
+ acrn_ioreq_put_client(client);
return 0;
}
@@ -562,7 +616,7 @@ int acrn_ioreq_attach_client(int client_id, bool check_kthread_stop)
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
if (!client) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EFAULT;
@@ -572,6 +626,7 @@ int acrn_ioreq_attach_client(int client_id, bool check_kthread_stop)
if (client->thread) {
pr_warn("vhm-ioreq: kthread already exist"
" for client %s\n", client->name);
+ acrn_ioreq_put_client(client);
return 0;
}
client->thread = kthread_run(ioreq_client_thread,
@@ -581,6 +636,7 @@ int acrn_ioreq_attach_client(int client_id, bool check_kthread_stop)
if (IS_ERR(client->thread)) {
pr_err("vhm-ioreq: failed to run kthread "
"for client %s\n", client->name);
+ acrn_ioreq_put_client(client);
return -ENOMEM;
}
clear_bit(IOREQ_CLIENT_EXIT, &client->flags);
@@ -603,10 +659,12 @@ int acrn_ioreq_attach_client(int client_id, bool check_kthread_stop)
if (is_destroying(client)) {
set_bit(IOREQ_CLIENT_EXIT, &client->flags);
+ acrn_ioreq_put_client(client);
return 1;
}
}
+ acrn_ioreq_put_client(client);
return 0;
}
EXPORT_SYMBOL_GPL(acrn_ioreq_attach_client);
@@ -619,7 +677,7 @@ void acrn_ioreq_intercept_bdf(int client_id, int bus, int dev, int func)
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
if (!client) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return;
@@ -628,6 +686,7 @@ void acrn_ioreq_intercept_bdf(int client_id, int bus, int dev, int func)
client->pci_bus = bus;
client->pci_dev = dev;
client->pci_func = func;
+ acrn_ioreq_put_client(client);
}
EXPORT_SYMBOL_GPL(acrn_ioreq_intercept_bdf);
@@ -639,7 +698,7 @@ void acrn_ioreq_unintercept_bdf(int client_id)
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
if (!client) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return;
@@ -648,6 +707,7 @@ void acrn_ioreq_unintercept_bdf(int client_id)
client->pci_bus = -1;
client->pci_dev = -1;
client->pci_func = -1;
+ acrn_ioreq_put_client(client);
}
static void acrn_ioreq_notify_client(struct ioreq_client *client)
@@ -802,22 +862,24 @@ static struct ioreq_client *acrn_ioreq_find_client_by_request(struct vhm_vm *vm,
{
struct list_head *pos, *range_pos;
struct ioreq_client *client;
- struct ioreq_client *target_client = NULL, *fallback_client = NULL;
+ int target_client,fallback_client;
struct ioreq_range *range;
bool found = false;
+ target_client = 0;
+ fallback_client = 0;
spin_lock(&vm->ioreq_client_lock);
list_for_each(pos, &vm->ioreq_client_list) {
client = container_of(pos, struct ioreq_client, list);
if (client->fallback) {
- fallback_client = client;
+ fallback_client = client->id;
continue;
}
if (req->type == REQ_PCICFG) {
if (bdf_match(vm, client)) { /* bdf match client */
- target_client = client;
+ target_client = client->id;
break;
} else /* other or fallback client */
continue;
@@ -829,7 +891,7 @@ static struct ioreq_client *acrn_ioreq_find_client_by_request(struct vhm_vm *vm,
container_of(range_pos, struct ioreq_range, list);
if (req_in_range(range, req)) {
found = true;
- target_client = client;
+ target_client = client->id;
break;
}
}
@@ -840,11 +902,11 @@ static struct ioreq_client *acrn_ioreq_find_client_by_request(struct vhm_vm *vm,
}
spin_unlock(&vm->ioreq_client_lock);
- if (target_client)
- return target_client;
+ if (target_client > 0)
+ return acrn_ioreq_get_client(target_client);
- if (fallback_client)
- return fallback_client;
+ if (fallback_client > 0)
+ return acrn_ioreq_get_client(fallback_client);
return NULL;
}
@@ -875,6 +937,7 @@ int acrn_ioreq_distribute_request(struct vhm_vm *vm)
req->client = client->id;
atomic_set(&req->processed, REQ_STATE_PROCESSING);
set_bit(i, client->ioreqs_map);
+ acrn_ioreq_put_client(client);
}
}
}
@@ -894,12 +957,13 @@ int acrn_ioreq_complete_request(int client_id, uint64_t vcpu,
struct vhm_request *vhm_req)
{
struct ioreq_client *client;
+ int ret;
if (client_id < 0 || client_id >= MAX_CLIENT) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EINVAL;
}
- client = clients[client_id];
+ client = acrn_ioreq_get_client(client_id);
if (!client) {
pr_err("vhm-ioreq: no client for id %d\n", client_id);
return -EINVAL;
@@ -911,7 +975,10 @@ int acrn_ioreq_complete_request(int client_id, uint64_t vcpu,
vhm_req += vcpu;
}
- return ioreq_complete_request(client->vmid, vcpu, vhm_req);
+ ret = ioreq_complete_request(client->vmid, vcpu, vhm_req);
+ acrn_ioreq_put_client(client);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(acrn_ioreq_complete_request);
@@ -928,7 +995,7 @@ unsigned int vhm_dev_poll(struct file *filep, poll_table *wait)
return ret;
}
- fallback_client = clients[vm->ioreq_fallback_client];
+ fallback_client = acrn_ioreq_get_client(vm->ioreq_fallback_client);
if (!fallback_client) {
pr_err("vhm-ioreq: no client for id %d\n",
vm->ioreq_fallback_client);
@@ -940,6 +1007,8 @@ unsigned int vhm_dev_poll(struct file *filep, poll_table *wait)
is_destroying(fallback_client))
ret = POLLIN | POLLRDNORM;
+ acrn_ioreq_put_client(fallback_client);
+
return ret;
}
@@ -969,9 +1038,6 @@ int acrn_ioreq_init(struct vhm_vm *vm, unsigned long vma)
return -EFAULT;
}
- /* reserve 0, let client_id start from 1 */
- set_bit(0, client_bitmap);
-
pr_info("vhm-ioreq: init request buffer @ %p!\n",
vm->req_buf);
@@ -982,10 +1048,15 @@ void acrn_ioreq_free(struct vhm_vm *vm)
{
struct list_head *pos, *tmp;
+ /* When acrn_ioreq_destory_client is called, it will be released
+ * and removed from vm->ioreq_client_list.
+ * The below is used to assure that the client is still released even when
+ * it is not called.
+ */
list_for_each_safe(pos, tmp, &vm->ioreq_client_list) {
struct ioreq_client *client =
container_of(pos, struct ioreq_client, list);
- acrn_ioreq_destroy_client_pervm(client, vm);
+ acrn_ioreq_destroy_client(client->id);
}
if (vm->req_buf && vm->pg) {
@@ -994,3 +1065,8 @@ void acrn_ioreq_free(struct vhm_vm *vm)
vm->req_buf = NULL;
}
}
+
+void acrn_ioreq_driver_init()
+{
+ idr_init(&idr_client);
+}
diff --git a/include/linux/vhm/acrn_vhm_ioreq.h b/include/linux/vhm/acrn_vhm_ioreq.h
index 70349223c55b..401bcdc85460 100644
--- a/include/linux/vhm/acrn_vhm_ioreq.h
+++ b/include/linux/vhm/acrn_vhm_ioreq.h
@@ -198,4 +198,5 @@ void acrn_ioreq_free(struct vhm_vm *vm);
int acrn_ioreq_create_fallback_client(unsigned long vmid, char *name);
unsigned int vhm_dev_poll(struct file *filep, poll_table *wait);
+void acrn_ioreq_driver_init(void);
#endif
--
https://clearlinux.org