summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authordimitri staessens <dimitri.staessens@intec.ugent.be>2016-10-23 16:00:33 +0200
committerdimitri staessens <dimitri.staessens@intec.ugent.be>2016-10-23 16:00:33 +0200
commit5a2e524418154deab5cac0e65d48a8391c860eea (patch)
tree69170f9ac6b659233fb30c2bdf3429c801a5d1aa /src
parentcd9a137216afe11620262e1df763b1c757cebd16 (diff)
downloadouroboros-5a2e524418154deab5cac0e65d48a8391c860eea.tar.gz
ouroboros-5a2e524418154deab5cac0e65d48a8391c860eea.zip
ipcpd: Improve locking in ipcp-data
This solves some potential data races in the shims.
Diffstat (limited to 'src')
-rw-r--r--src/ipcpd/ipcp-data.c112
-rw-r--r--src/ipcpd/ipcp-data.h6
2 files changed, 76 insertions, 42 deletions
diff --git a/src/ipcpd/ipcp-data.c b/src/ipcpd/ipcp-data.c
index c4838d3a..ed7e578d 100644
--- a/src/ipcpd/ipcp-data.c
+++ b/src/ipcpd/ipcp-data.c
@@ -32,6 +32,7 @@
#include <string.h>
#include <stdlib.h>
+#include <assert.h>
struct reg_entry {
struct list_head list;
@@ -50,6 +51,8 @@ static struct reg_entry * reg_entry_create(char * name)
if (entry == NULL)
return NULL;
+ assert(name);
+
entry->name = name;
if (entry->name == NULL)
return NULL;
@@ -59,11 +62,11 @@ static struct reg_entry * reg_entry_create(char * name)
static void reg_entry_destroy(struct reg_entry * entry)
{
- if (entry == NULL)
- return;
+ assert(entry);
if (entry->name != NULL)
free(entry->name);
+
free(entry);
}
@@ -74,6 +77,8 @@ static struct dir_entry * dir_entry_create(char * ap_name,
if (entry == NULL)
return NULL;
+ assert(ap_name);
+
entry->addr = addr;
entry->ap_name = ap_name;
if (entry->ap_name == NULL)
@@ -84,11 +89,11 @@ static struct dir_entry * dir_entry_create(char * ap_name,
static void dir_entry_destroy(struct dir_entry * entry)
{
- if (entry == NULL)
- return;
+ assert(entry);
if (entry->ap_name != NULL)
free(entry->ap_name);
+
free(entry);
}
@@ -116,9 +121,9 @@ struct ipcp_data * ipcp_data_init(struct ipcp_data * dst,
INIT_LIST_HEAD(&dst->registry);
INIT_LIST_HEAD(&dst->directory);
- /* init the mutexes */
- pthread_mutex_init(&dst->reg_lock, NULL);
- pthread_mutex_init(&dst->dir_lock, NULL);
+ /* init the locks */
+ pthread_rwlock_init(&dst->reg_lock, NULL);
+ pthread_rwlock_init(&dst->dir_lock, NULL);
return dst;
}
@@ -127,6 +132,9 @@ static void clear_registry(struct ipcp_data * data)
{
struct list_head * h;
struct list_head * t;
+
+ assert(data);
+
list_for_each_safe(h, t, &data->registry) {
struct reg_entry * e = list_entry(h, struct reg_entry, list);
list_del(&e->list);
@@ -138,6 +146,9 @@ static void clear_directory(struct ipcp_data * data)
{
struct list_head * h;
struct list_head * t;
+
+ assert(data);
+
list_for_each_safe(h, t, &data->directory) {
struct dir_entry * e = list_entry(h, struct dir_entry, list);
list_del(&e->list);
@@ -150,21 +161,23 @@ void ipcp_data_destroy(struct ipcp_data * data)
if (data == NULL)
return;
- pthread_mutex_lock(&data->reg_lock);
- pthread_mutex_lock(&data->dir_lock);
-
/* clear the lists */
+ pthread_rwlock_wrlock(&data->reg_lock);
+
clear_registry(data);
+
+ pthread_rwlock_unlock(&data->reg_lock);
+ pthread_rwlock_wrlock(&data->dir_lock);
+
clear_directory(data);
+ pthread_rwlock_unlock(&data->dir_lock);
+
if (data->dif_name != NULL)
free(data->dif_name);
- pthread_mutex_unlock(&data->dir_lock);
- pthread_mutex_unlock(&data->reg_lock);
-
- pthread_mutex_destroy(&data->dir_lock);
- pthread_mutex_destroy(&data->reg_lock);
+ pthread_rwlock_destroy(&data->dir_lock);
+ pthread_rwlock_destroy(&data->reg_lock);
free(data);
}
@@ -175,6 +188,10 @@ static struct reg_entry * find_reg_entry_by_name(struct ipcp_data * data,
const char * name)
{
struct list_head * h;
+
+ assert(data);
+ assert(name);
+
list_for_each(h, &data->registry) {
struct reg_entry * e = list_entry(h, struct reg_entry, list);
if (!strcmp(e->name, name))
@@ -214,7 +231,15 @@ static struct dir_entry * find_dir_entry_any(struct ipcp_data * data,
bool ipcp_data_is_in_directory(struct ipcp_data * data,
const char * ap_name)
{
- return find_dir_entry_any(data, ap_name) != NULL;
+ bool ret = false;
+
+ pthread_rwlock_rdlock(&data->dir_lock);
+
+ ret = (find_dir_entry_any(data, ap_name) != NULL);
+
+ pthread_rwlock_unlock(&data->dir_lock);
+
+ return ret;
}
int ipcp_data_del_reg_entry(struct ipcp_data * data,
@@ -224,19 +249,19 @@ int ipcp_data_del_reg_entry(struct ipcp_data * data,
if (data == NULL)
return -1;
- pthread_mutex_lock(&data->reg_lock);
+ pthread_rwlock_wrlock(&data->reg_lock);
e = find_reg_entry_by_name(data, name);
if (e == NULL) {
- pthread_mutex_unlock(&data->reg_lock);
+ pthread_rwlock_unlock(&data->reg_lock);
return 0; /* nothing to do */
}
list_del(&e->list);
- reg_entry_destroy(e);
+ pthread_rwlock_unlock(&data->reg_lock);
- pthread_mutex_unlock(&data->reg_lock);
+ reg_entry_destroy(e);
return 0;
}
@@ -249,19 +274,19 @@ int ipcp_data_del_dir_entry(struct ipcp_data * data,
if (data == NULL)
return -1;
- pthread_mutex_lock(&data->dir_lock);
+ pthread_rwlock_wrlock(&data->dir_lock);
e = find_dir_entry(data, ap_name, addr);
if (e == NULL) {
- pthread_mutex_unlock(&data->dir_lock);
+ pthread_rwlock_unlock(&data->dir_lock);
return 0; /* nothing to do */
}
list_del(&e->list);
- dir_entry_destroy(e);
+ pthread_rwlock_unlock(&data->dir_lock);
- pthread_mutex_unlock(&data->dir_lock);
+ dir_entry_destroy(e);
return 0;
}
@@ -274,22 +299,22 @@ int ipcp_data_add_reg_entry(struct ipcp_data * data,
if (data == NULL || name == NULL)
return -1;
- pthread_mutex_lock(&data->reg_lock);
+ pthread_rwlock_wrlock(&data->reg_lock);
if (find_reg_entry_by_name(data, name)) {
- pthread_mutex_unlock(&data->reg_lock);
- return -2;
+ pthread_rwlock_unlock(&data->reg_lock);
+ return -1;
}
entry = reg_entry_create(name);
if (entry == NULL) {
- pthread_mutex_unlock(&data->reg_lock);
+ pthread_rwlock_unlock(&data->reg_lock);
return -1;
}
list_add(&entry->list, &data->registry);
- pthread_mutex_unlock(&data->reg_lock);
+ pthread_rwlock_unlock(&data->reg_lock);
return 0;
}
@@ -303,22 +328,22 @@ int ipcp_data_add_dir_entry(struct ipcp_data * data,
if (data == NULL || ap_name == NULL)
return -1;
- pthread_mutex_lock(&data->dir_lock);
+ pthread_rwlock_wrlock(&data->dir_lock);
if (find_dir_entry(data, ap_name, addr) != NULL) {
- pthread_mutex_unlock(&data->dir_lock);
- return -2;
+ pthread_rwlock_unlock(&data->dir_lock);
+ return -1;
}
entry = dir_entry_create(ap_name, addr);
if (entry == NULL) {
- pthread_mutex_unlock(&data->dir_lock);
+ pthread_rwlock_unlock(&data->dir_lock);
return -1;
}
list_add(&entry->list,&data->directory);
- pthread_mutex_unlock(&data->dir_lock);
+ pthread_rwlock_unlock(&data->dir_lock);
return 0;
}
@@ -326,7 +351,18 @@ int ipcp_data_add_dir_entry(struct ipcp_data * data,
bool ipcp_data_is_in_registry(struct ipcp_data * data,
const char * ap_name)
{
- return find_reg_entry_by_name(data, ap_name) != NULL;
+ bool ret = false;
+
+ if (data == NULL || ap_name == NULL)
+ return false;
+
+ pthread_rwlock_rdlock(&data->reg_lock);
+
+ ret = (find_reg_entry_by_name(data, ap_name) != NULL);
+
+ pthread_rwlock_unlock(&data->reg_lock);
+
+ return ret;
}
uint64_t ipcp_data_get_addr(struct ipcp_data * data,
@@ -335,18 +371,18 @@ uint64_t ipcp_data_get_addr(struct ipcp_data * data,
struct dir_entry * entry;
uint64_t addr;
- pthread_mutex_lock(&data->dir_lock);
+ pthread_rwlock_rdlock(&data->dir_lock);
entry = find_dir_entry_any(data, ap_name);
if (entry == NULL) {
- pthread_mutex_unlock(&data->dir_lock);
+ pthread_rwlock_unlock(&data->dir_lock);
return 0; /* undefined behaviour, 0 may be a valid address */
}
addr = entry->addr;
- pthread_mutex_unlock(&data->dir_lock);
+ pthread_rwlock_unlock(&data->dir_lock);
return addr;
}
diff --git a/src/ipcpd/ipcp-data.h b/src/ipcpd/ipcp-data.h
index a9a637f0..4472a1d7 100644
--- a/src/ipcpd/ipcp-data.h
+++ b/src/ipcpd/ipcp-data.h
@@ -36,12 +36,10 @@ struct ipcp_data {
char * dif_name;
struct list_head registry;
- pthread_mutex_t reg_lock;
+ pthread_rwlock_t reg_lock;
struct list_head directory;
- pthread_mutex_t dir_lock;
-
- pthread_mutex_t lock;
+ pthread_rwlock_t dir_lock;
};
struct ipcp_data * ipcp_data_create(void);