fraud_detection: Fix incomplete locking logic

The get_item() function results were insufficiently guarded, such
that the SIP workers may concurrently perform insert operations on the
same map object, possibly leading to shm corruption.

Fixes #1389

(cherry picked from commit ccdc8d2)

modules/fraud_detection/frd_hashmap.c

@@ -32,56 +32,48 @@
 
 int init_hash_map(hash_map_t *hm)
 {
+	unsigned int i;
+
 	hm->buckets = shm_malloc(hm->size * sizeof(hash_bucket_t));
 	if (hm->buckets == NULL) {
 		LM_ERR("No more shm memory\n");
 		return -1;
 	}
 
-	unsigned int i;
-
 	for (i = 0; i < hm->size; ++i) {
 		hm->buckets[i].items = map_create(AVLMAP_SHARED);
-		hm->buckets[i].lock = lock_init_rw();
-		if (hm->buckets[i].lock == NULL) {
+		if (!hm->buckets[i].items) {
+			LM_ERR("oom\n");
+			return -1;
+		}
+
+		hm->buckets[i].lock = lock_alloc();
+		if (!hm->buckets[i].lock) {
 			LM_ERR("cannot init lock\n");
 			shm_free(hm->buckets);
 			return -1;
 		}
+
+		if (!lock_init(hm->buckets[i].lock)) {
+			lock_dealloc(hm->buckets[i].lock);
+			shm_free(hm->buckets);
+			LM_ERR("faled to init lock\n");
+			return -1;
+		}
 	}
 
 	return 0;
 }
 
-void** get_item (hash_map_t *hm, str key)
-{
-	unsigned int hash = core_hash(&key, NULL, hm->size);
-
-	lock_start_read(hm->buckets[hash].lock);
-	void **find_res = map_find(hm->buckets[hash].items, key);
-	lock_stop_read(hm->buckets[hash].lock);
-	if (find_res) {
-		return find_res;
-	}
-	else {
-		lock_start_write(hm->buckets[hash].lock);
-		find_res = map_get(hm->buckets[hash].items, key);
-		lock_stop_write(hm->buckets[hash].lock);
-		return find_res;
-	}
-}
-
 void free_hash_map(hash_map_t* hm, void (*value_destroy_func)(void *))
 {
 	unsigned int i;
 
 	/* no need for locking -- if there were, the readers would insta-die */
 	for (i = 0; i < hm->size; ++i) {
 		map_destroy(hm->buckets[i].items, value_destroy_func);
-		lock_destroy_rw(hm->buckets[i].lock);
+		lock_dealloc(hm->buckets[i].lock);
 	}
 
 	shm_free(hm->buckets);
 }
-
-

modules/fraud_detection/frd_hashmap.h

@@ -32,7 +32,7 @@
 
 typedef struct {
 	map_t items;
-	rw_lock_t   *lock;
+	gen_lock_t   *lock;
 } hash_bucket_t;
 
 typedef struct {
@@ -43,7 +43,11 @@ typedef struct {
 
 
 int init_hash_map(hash_map_t* hm);
-void** get_item (hash_map_t *hm, str key);
+static inline void **get_item(hash_map_t *hm, str key, unsigned int hash)
+{
+	return map_get(hm->buckets[hash].items, key);
+}
+
 void free_hash_map(hash_map_t* hm, void (*value_destroy_func)(void *));
 
 #endif

modules/fraud_detection/frd_stats.c

@@ -58,72 +58,103 @@ int init_stats_table(void)
 
 frd_stats_entry_t* get_stats(str user, str prefix, str *shm_user)
 {
+	unsigned int hash = core_hash(&user, NULL, FRD_USER_HASH_SIZE);
+
+	lock_get(stats_table.buckets[hash].lock);
+
 	/* First go one level below using the user key */
 	frd_users_map_item_t **hm =
-		(frd_users_map_item_t **)get_item(&stats_table, user);
+		(frd_users_map_item_t **)get_item(&stats_table, user, hash);
 
 	if (*hm == NULL) {
 		/* First time the user is seen, we must create a hashmap */
 		*hm = shm_malloc(sizeof(frd_users_map_item_t));
 		if (*hm == NULL) {
+			lock_release(stats_table.buckets[hash].lock);
 			LM_ERR("no more shm memory\n");
 			return NULL;
 		}
 
 		(*hm)->numbers_hm.size = FRD_PREFIX_HASH_SIZE;
 		if (init_hash_map(&(*hm)->numbers_hm) != 0) {
+			shm_free(*hm); *hm = NULL;
+			lock_release(stats_table.buckets[hash].lock);
 			LM_ERR("cannot init hashmap\n");
-			shm_free(*hm);
 			return NULL;
 		}
 
 		if (shm_str_dup(&(*hm)->user, &user) != 0) {
 			free_hash_map(&(*hm)->numbers_hm, destroy_stats_entry);
-			shm_free(*hm);
+			shm_free(*hm); *hm = NULL;
+			lock_release(stats_table.buckets[hash].lock);
+			LM_ERR("oom\n");
 			return NULL;
 		}
 	}
 
+	lock_release(stats_table.buckets[hash].lock);
+
 	if (shm_user)
 		*shm_user = (*hm)->user;
 
+	hash = core_hash(&prefix, NULL, FRD_PREFIX_HASH_SIZE);
+	lock_get((*hm)->numbers_hm.buckets[hash].lock);
+
 	frd_stats_entry_t **stats_entry =
-		(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix);
+		(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix, hash);
 	if (*stats_entry == NULL) {
 		/* First time the prefix is seen for this user */
 		*stats_entry = shm_malloc(sizeof(frd_stats_entry_t));
 		if (*stats_entry == NULL) {
+			lock_release((*hm)->numbers_hm.buckets[hash].lock);
 			LM_ERR("no more shm memory\n");
 			return NULL;
 		}
 
 		/* Now init the auxiliary info for a stats structure */
 		if (!lock_init(&(*stats_entry)->lock)) {
+			shm_free(*stats_entry); *stats_entry = NULL;
+			lock_release((*hm)->numbers_hm.buckets[hash].lock);
 			LM_ERR ("cannot init lock\n");
-			shm_free(*stats_entry);
 			return NULL;
 		}
-		memset(&((*stats_entry)->stats), 0, sizeof(frd_stats_t));
+		memset(&(*stats_entry)->stats, 0, sizeof(frd_stats_t));
 	}
 
+	lock_release((*hm)->numbers_hm.buckets[hash].lock);
+
 	return *stats_entry;
 }
 
 
 int stats_exist(str user, str prefix)
 {
+	unsigned int hash = core_hash(&user, NULL, FRD_USER_HASH_SIZE);
+
+	lock_get(stats_table.buckets[hash].lock);
+
 	/* First go one level below using the user key */
 	frd_users_map_item_t **hm =
-		(frd_users_map_item_t **)get_item(&stats_table, user);
+		(frd_users_map_item_t **)get_item(&stats_table, user, hash);
 
-	if (*hm == NULL)
+	if (*hm == NULL) {
+		lock_release(stats_table.buckets[hash].lock);
 		return 0;
+	}
+
+	lock_release(stats_table.buckets[hash].lock);
+
+	hash = core_hash(&prefix, NULL, FRD_PREFIX_HASH_SIZE);
+	lock_get((*hm)->numbers_hm.buckets[hash].lock);
 
 	frd_stats_entry_t **stats_entry =
-		(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix);
-	if (*stats_entry == NULL)
+		(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix, hash);
+	if (*stats_entry == NULL) {
+		lock_release((*hm)->numbers_hm.buckets[hash].lock);
 		return 0;
+	}
 
+	lock_release((*hm)->numbers_hm.buckets[hash].lock);
 	return 1;
 }