dispatcher: Fix rare crash with 'pvar_algo_pattern' (part 2)

Commit fe1a50d was a quick-fix attempt, but the race condition was
still there, and continuing to cause rare crashes during 'ds_reload' if
using the 'pvar_algo_pattern' feature.

Long story short: pv_get_spec_value() on SHM-allocated structs has a
high risk of running into concurrency issues, depending on the spec in
use.  Here, the issue happened to be with the $stat(xxx) algo pattern,
where 2 or more SIP workers performed concurrent READ/WRITE on it,
leading to a crash.

Many thanks to Eric Tamme from Five9 for reporting & testing!

(cherry picked from commit 063435d)

Author: liviuchircu

modules/dispatcher/dispatch.c

@@ -368,6 +368,11 @@ int add_dest2list(int id, str uri, const struct socket_info *sock, str *comsock,
 		}
 	}
 
+	if (!lock_init(&dp->wlock)) {
+		LM_ERR("failed to init lock\n");
+		goto err;
+	}
+
 	/*
 	 * search the proper place based on priority
 	 * put them in reverse order, since they will be reindexed
@@ -643,6 +648,8 @@ int ds_pvar_algo(struct sip_msg *msg, ds_set_p set, ds_dest_p **sorted_set,
 	}
 
 	for (i = 0, cnt = 0; i < set->nr - (ds_use_default?1:0); i++) {
+		int locked = 0;
+
 		if ( !dst_is_active(set->dlist[i]) ) {
 			/* move to the end of the list */
 			sset[end_idx--] = &set->dlist[i];
@@ -657,15 +664,38 @@ int ds_pvar_algo(struct sip_msg *msg, ds_set_p set, ds_dest_p **sorted_set,
 					   set->dlist[i].uri.len, set->dlist[i].uri.s);
 				continue;
 			}
-			set->dlist[i].param = (void *)param;
+
+			/* concurrent access -- avoid SHM leak */
+			lock_get(&set->dlist[i].wlock);
+			if (set->dlist[i].param) {
+				shm_free(param);
+				param = (ds_pvar_param_p)set->dlist[i].param;
+			} else {
+				set->dlist[i].param = (void *)param;
+			}
+			lock_release(&set->dlist[i].wlock);
 		} else {
 			param = (ds_pvar_param_p)set->dlist[i].param;
 		}
+
+		/* until the underlying (stat *) struct is created, we cannot
+		 * perform READ/WRITE against this pv_spec_t concurrently */
+		if (!pv_has_dname(&param->pvar)) {
+			locked = 1;
+			lock_get(&set->dlist[i].wlock);
+		}
+
 		if (pv_get_spec_value(msg, &param->pvar, &val) < 0) {
+			if (locked)
+				lock_release(&set->dlist[i].wlock);
 			LM_ERR("cannot get spec value for spec %.*s\n",
 				   set->dlist[i].uri.len, set->dlist[i].uri.s);
 			continue;
 		}
+
+		if (locked)
+			lock_release(&set->dlist[i].wlock);
+
 		if (!(val.flags & PV_VAL_NULL)) {
 			if (!(val.flags & PV_VAL_INT)) {
 				/* last attempt to retrieve value */

modules/dispatcher/dispatch.h

@@ -67,7 +67,7 @@ typedef struct _ds_dest
 	str attrs;
 	str script_attrs;
 	str description;
-	int flags;
+	int flags;      /* e.g. DS_INACTIVE_DST, etc. */
 	unsigned short weight;    /* dynamic weight - may change at runtime */
 	unsigned short rr_count; /* times it was chosen in a row for weighted round-robin */
 	unsigned short running_weight;
@@ -83,6 +83,7 @@ typedef struct _ds_dest
 	void *param;
 	int route_algo_value;
 	fs_evs *fs_sock;
+	gen_lock_t wlock;      /* serialize any concurrent R/W on this dest */
 	struct _ds_dest *next;
 } ds_dest_t, *ds_dest_p;
 

modules/statistics/statistics.c

@@ -806,8 +806,8 @@ static inline int get_stat_name(struct sip_msg* msg, pv_name_t *name,
 			//shm_free(name->u.isname.name.s.s);
 			name->u.isname.name.s.s = NULL;
 			name->u.isname.name.s.len = 0;
-			name->type = PV_NAME_PVAR;
 			name->u.dname = (void*)*stat;
+			name->type = PV_NAME_PVAR;
 		}
 	} else {
 		/* stat already found ! */