[sr-dev] git:5.3:93d06acf: ims_ipsec_pcscf: fixed defects and warnings:

Module: kamailio Branch: 5.3 Commit: 93d06acf1f1cc201ce3ca6970c2f73eec0e57d87 URL: https://github.com/kamailio/kamailio/commit/93d06acf1f1cc201ce3ca6970c2f73e… Author: Aleksandar Yosifov &lt;alexyosifov(a)gmail.com&gt; Committer: Henning Westerholt &lt;hw(a)skalatan.de&gt; Date: 2019-12-03T16:11:53+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. (cherry picked from commit 8973feb34ab5059966158b34d3d3e3deb2af1b23) --- Modified: src/modules/ims_ipsec_pcscf/cmd.c Modified: src/modules/ims_ipsec_pcscf/spi_gen.c --- Diff: https://github.com/kamailio/kamailio/commit/93d06acf1f1cc201ce3ca6970c2f73e… Patch: https://github.com/kamailio/kamailio/commit/93d06acf1f1cc201ce3ca6970c2f73e… --- diff --git a/src/modules/ims_ipsec_pcscf/cmd.c b/src/modules/ims_ipsec_pcscf/cmd.c index 11673582ad..c494941115 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; @@ -791,7 +791,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;