[sr-dev] git:4.0: tls: the pointer of cfg structure

Daniel-Constantin Mierla miconda at gmail.com
Thu Mar 6 11:59:56 CET 2014


Module: sip-router
Branch: 4.0
Commit: dfc8d544f7435e39771f8a39f349b969ba730944
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=dfc8d544f7435e39771f8a39f349b969ba730944

Author: Daniel-Constantin Mierla <miconda at gmail.com>
Committer: Daniel-Constantin Mierla <miconda at gmail.com>
Date:   Thu Mar  6 11:57:23 2014 +0100

tls: the pointer of cfg structure

- synchronize when updating the list of tls configs and their reference
  count
- with parts from patch by Ding Ma, FS#380, contributed under BSD
  license
- backport of 208934d47ac1585d42029234267bcceab40ae218

---

 modules/tls/tls_cfg.c    |    2 +-
 modules/tls/tls_cfg.h    |    2 +-
 modules/tls/tls_domain.c |    1 +
 modules/tls/tls_domain.h |    2 +-
 modules/tls/tls_mod.c    |    2 +-
 modules/tls/tls_rpc.c    |    2 ++
 modules/tls/tls_server.c |    9 +++------
 modules/tls/tls_util.c   |   11 +++++++----
 8 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/modules/tls/tls_cfg.c b/modules/tls/tls_cfg.c
index d1253af..dc118d8 100644
--- a/modules/tls/tls_cfg.c
+++ b/modules/tls/tls_cfg.c
@@ -74,7 +74,7 @@ struct cfg_group_tls default_tls_cfg = {
 	0 /* send_close_notify (off by default)*/
 };
 
-void* tls_cfg = &default_tls_cfg;
+volatile void* tls_cfg = &default_tls_cfg;
 
 
 /* if *to<0 to=default_val, else if to>max_val to=max_val */
diff --git a/modules/tls/tls_cfg.h b/modules/tls/tls_cfg.h
index 86b8816..6064182 100644
--- a/modules/tls/tls_cfg.h
+++ b/modules/tls/tls_cfg.h
@@ -101,7 +101,7 @@ struct cfg_group_tls {
 
 
 extern struct cfg_group_tls default_tls_cfg;
-extern void* tls_cfg;
+extern volatile void* tls_cfg;
 extern cfg_def_t tls_cfg_def[];
 
 
diff --git a/modules/tls/tls_domain.c b/modules/tls/tls_domain.c
index 2d49969..c06892c 100644
--- a/modules/tls/tls_domain.c
+++ b/modules/tls/tls_domain.c
@@ -118,6 +118,7 @@ void tls_free_cfg(tls_domains_cfg_t* cfg)
 	}
 	if (cfg->srv_default) tls_free_domain(cfg->srv_default);
 	if (cfg->cli_default) tls_free_domain(cfg->cli_default);
+	shm_free(cfg);
 }
 
 
diff --git a/modules/tls/tls_domain.h b/modules/tls/tls_domain.h
index e4d1319..2f5c72a 100644
--- a/modules/tls/tls_domain.h
+++ b/modules/tls/tls_domain.h
@@ -92,7 +92,7 @@ typedef struct tls_domains_cfg {
 	tls_domain_t* srv_list;    /**< Server domain list */
 	tls_domain_t* cli_list;    /**< Client domain list */
 	struct tls_domains_cfg* next; /**< Next element in the garbage list */
-	int ref_count;             /**< How many connections use this configuration */
+	volatile int ref_count;             /**< How many connections use this configuration */
 } tls_domains_cfg_t;
 
 
diff --git a/modules/tls/tls_mod.c b/modules/tls/tls_mod.c
index 69967da..bb5d3ad 100644
--- a/modules/tls/tls_mod.c
+++ b/modules/tls/tls_mod.c
@@ -300,7 +300,7 @@ static int mod_init(void)
 	}
 	/* declare configuration */
 	if (cfg_declare("tls", tls_cfg_def, &default_tls_cfg,
-							cfg_sizeof(tls), &tls_cfg)) {
+							cfg_sizeof(tls), (void **)&tls_cfg)) {
 		ERR("failed to register the configuration\n");
 		return -1;
 	}
diff --git a/modules/tls/tls_rpc.c b/modules/tls/tls_rpc.c
index 12a0711..b7b0a3c 100644
--- a/modules/tls/tls_rpc.c
+++ b/modules/tls/tls_rpc.c
@@ -81,8 +81,10 @@ static void tls_reload(rpc_t* rpc, void* ctx)
 	}
 
 	DBG("TLS configuration successfuly loaded");
+	lock_get(tls_domains_cfg_lock);
 	cfg->next = (*tls_domains_cfg);
 	*tls_domains_cfg = cfg;
+	lock_release(tls_domains_cfg_lock);
 	return;
 
  error:
diff --git a/modules/tls/tls_server.c b/modules/tls/tls_server.c
index f71956d..353e4e9 100644
--- a/modules/tls/tls_server.c
+++ b/modules/tls/tls_server.c
@@ -151,13 +151,9 @@ static int tls_complete_init(struct tcp_connection* c)
 		goto error2;
 	}
 	     /* Get current TLS configuration and increase reference
-	      * count immediately. There is no need to lock the structure
-	      * here, because it does not get deleted immediately. When
-	      * SER reloads TLS configuration it will put the old configuration
-	      * on a garbage queue and delete it later, so we know here that
-	      * the pointer we get from *tls_domains_cfg will be valid for a while,
-		  * at least by the time this function finishes
+	      * count immediately.
 	      */
+	lock_get(tls_domains_cfg_lock);
 	cfg = *tls_domains_cfg;
 
 	     /* Increment the reference count in the configuration structure, this
@@ -165,6 +161,7 @@ static int tls_complete_init(struct tcp_connection* c)
 	      * not get deleted if there are still connection referencing its SSL_CTX
 	      */
 	cfg->ref_count++;
+	lock_release(tls_domains_cfg_lock);
 
 	if (c->flags & F_CONN_PASSIVE) {
 		state=S_TLS_ACCEPTING;
diff --git a/modules/tls/tls_util.c b/modules/tls/tls_util.c
index a0897fc..8ec8892 100644
--- a/modules/tls/tls_util.c
+++ b/modules/tls/tls_util.c
@@ -74,7 +74,7 @@ int shm_asciiz_dup(char** dest, char* val)
  */
 void collect_garbage(void)
 {
-	tls_domains_cfg_t* prev, *cur;
+	tls_domains_cfg_t* prev, *cur, *next;
 
 	     /* Make sure we do not run two garbage collectors
 	      * at the same time
@@ -88,14 +88,17 @@ void collect_garbage(void)
 	cur = (*tls_domains_cfg)->next;
 
 	while(cur) {
+		next = cur->next;
 		if (cur->ref_count == 0) {
-			     /* Not referenced by any existing connection */
+			/* Not referenced by any existing connection */
 			prev->next = cur->next;
 			tls_free_cfg(cur);
+		} else {
+			/* Only update prev if we didn't remove cur */
+			prev = cur;
 		}
 
-		prev = cur;
-		cur = cur->next;
+		cur = next;
 	}
 
 	lock_release(tls_domains_cfg_lock);




More information about the sr-dev mailing list