[sr-dev] git:master:8973feb3: ims_ipsec_pcscf: fixed defects and warnings:

[Henning Westerholt]

Module: kamailio
Branch: master
Commit: 8973feb34ab5059966158b34d3d3e3deb2af1b23
URL: https://github.com/kamailio/kamailio/commit/8973feb34ab5059966158b34d3d3e3deb2af1b23

Author: Aleksandar Yosifov <alexyosifov at gmail.com>
Committer: Henning Westerholt <hw at skalatan.de>
Date: 2019-12-03T16:10:39+01:00

ims_ipsec_pcscf: fixed defects and warnings:

- fixed a defect reported from Coverity Scan - Concurrent data access
  violations in spi_gen.c for spi_data->spi_val.
- in create_ipsec_tunnel() return -1 when unable to convert ip address.
- in ipsec_create() go to cleanup instead return -1 when get request
  for reply fails. Fixed debug message for that error.

---

Modified: src/modules/ims_ipsec_pcscf/cmd.c
Modified: src/modules/ims_ipsec_pcscf/spi_gen.c

---

Diff:  https://github.com/kamailio/kamailio/commit/8973feb34ab5059966158b34d3d3e3deb2af1b23.diff
Patch: https://github.com/kamailio/kamailio/commit/8973feb34ab5059966158b34d3d3e3deb2af1b23.patch

---

diff --git a/src/modules/ims_ipsec_pcscf/cmd.c b/src/modules/ims_ipsec_pcscf/cmd.c
index b72d57a0ad..53b09c56fd 100644
--- a/src/modules/ims_ipsec_pcscf/cmd.c
+++ b/src/modules/ims_ipsec_pcscf/cmd.c
@@ -339,18 +339,18 @@ static int create_ipsec_tunnel(const struct ip_addr *remote_addr, ipsec_t* s)
         if(str2ipbuf(&ipsec_listen_addr, &ipsec_addr) < 0){
             LM_ERR("Unable to convert ipsec addr4 [%.*s]\n", ipsec_listen_addr.len, ipsec_listen_addr.s);
             close_mnl_socket(sock);
-            return 0;
+            return -1;
         }
     } else if(remote_addr->af == AF_INET6){
         if(str2ip6buf(&ipsec_listen_addr6, &ipsec_addr) < 0){
             LM_ERR("Unable to convert ipsec addr6 [%.*s]\n", ipsec_listen_addr6.len, ipsec_listen_addr6.s);
             close_mnl_socket(sock);
-            return 0;
+            return -1;
         }
     } else {
         LM_ERR("Unsupported AF %d\n", remote_addr->af);
         close_mnl_socket(sock);
-        return 0;
+        return -1;
     }
 
     //Convert to char* for logging
@@ -574,8 +574,8 @@ int ipsec_create(struct sip_msg* m, udomain_t* d)
     // Get request from reply
     struct cell *t = tmb.t_gett();
     if (!t || t == (void*) -1) {
-        LM_ERR("fill_contact(): Reply without transaction\n");
-        return -1;
+        LM_ERR("Reply without transaction\n");
+        goto cleanup;
     }
 
     struct sip_msg* req = t->uas.request;
@@ -792,7 +792,8 @@ int ipsec_forward(struct sip_msg* m, udomain_t* d)
         t->uas.response.dst = dst_info;
     }
 
-    LM_DBG("Destination changed to [%d://%.*s]\n", dst_info.proto, m->dst_uri.len, m->dst_uri.s);
+	LM_DBG("Destination changed to [%d://%.*s], from [%d:%d]\n", dst_info.proto, m->dst_uri.len, m->dst_uri.s,
+			dst_info.send_sock->proto, dst_info.send_sock->port_no);
 
     ret = IPSEC_CMD_SUCCESS; // all good, return SUCCESS
 
diff --git a/src/modules/ims_ipsec_pcscf/spi_gen.c b/src/modules/ims_ipsec_pcscf/spi_gen.c
index 5e329028c9..c0c4fc4c3e 100644
--- a/src/modules/ims_ipsec_pcscf/spi_gen.c
+++ b/src/modules/ims_ipsec_pcscf/spi_gen.c
@@ -60,11 +60,17 @@ int init_spi_gen(uint32_t start_val, uint32_t range)
         return 5;
     }
 
+	if(pthread_mutex_lock(&spi_data->spis_mut) != 0){
+		return 6;
+	}
+
     spi_data->used_spis = create_list();
 
     spi_data->spi_val = spi_data->min_spi = start_val;
     spi_data->max_spi = start_val + range;
 
+	pthread_mutex_unlock(&spi_data->spis_mut);
+
     return 0;
 }
 
@@ -74,14 +80,14 @@ uint32_t acquire_spi()
 		return 0;
 	}
 
+	if(pthread_mutex_lock(&spi_data->spis_mut) != 0){
+		return 0;
+	}
+
     //save the initial value for the highly unlikely case where there are no free SPIs
 	uint32_t initial_val = spi_data->spi_val;
     uint32_t ret = 0; // by default return invalid SPI
 
-	if(pthread_mutex_lock(&spi_data->spis_mut) != 0){
-        return ret;
-    }
-
     while(1) {
 		if(spi_in_list(&spi_data->used_spis, spi_data->spi_val) == 0){
 			ret = spi_data->spi_val;