From 4de841c26b7208d5395da349ea16c937b1361414 Mon Sep 17 00:00:00 2001 From: Sander Vrijders Date: Tue, 29 Aug 2017 13:14:25 +0200 Subject: lib, ipcpd, irmd: Fix bugs reported by static analysis This fixes several bugs as reported by the clang static analyzer. --- src/ipcpd/normal/dht.c | 3 ++- src/ipcpd/normal/pol/graph.c | 5 +++-- src/ipcpd/shim-eth-llc/main.c | 6 ++++-- src/ipcpd/shim-udp/main.c | 6 ++---- src/irmd/main.c | 11 ++++++++++ src/irmd/registry.c | 50 +++++++++++++++++++++++++++---------------- src/lib/btree.c | 10 +++++++-- src/lib/cdap.c | 1 + src/lib/frct_pci.c | 4 ++-- src/lib/irm.c | 1 - src/lib/rib.c | 4 +++- src/lib/shm_rdrbuff.c | 8 +++++-- 12 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/ipcpd/normal/dht.c b/src/ipcpd/normal/dht.c index b5d52d93..b5bac913 100644 --- a/src/ipcpd/normal/dht.c +++ b/src/ipcpd/normal/dht.c @@ -745,7 +745,8 @@ static void lookup_update(struct dht * dht, } else { struct contact * d; list_add_tail(&c->next, p); - d = list_last_entry(&lu->contacts, struct contact, next); + d = list_last_entry(&lu->contacts, + struct contact, next); list_del(&d->next); contact_destroy(d); } diff --git a/src/ipcpd/normal/pol/graph.c b/src/ipcpd/normal/pol/graph.c index 3611f0b0..9901fbaa 100644 --- a/src/ipcpd/normal/pol/graph.c +++ b/src/ipcpd/normal/pol/graph.c @@ -454,9 +454,10 @@ ssize_t graph_routing_table(struct graph * graph, pthread_mutex_lock(&graph->lock); - if (graph->nr_vertices == 0) { + /* We need at least 2 vertices for a table */ + if (graph->nr_vertices < 2) { pthread_mutex_unlock(&graph->lock); - return 0; + return -1; } prevs = dijkstra(graph, s_addr); diff --git a/src/ipcpd/shim-eth-llc/main.c b/src/ipcpd/shim-eth-llc/main.c index 3d186d7a..e5f0cba8 100644 --- a/src/ipcpd/shim-eth-llc/main.c +++ b/src/ipcpd/shim-eth-llc/main.c @@ -166,11 +166,13 @@ static int eth_llc_data_init(void) int ret = -ENOMEM; pthread_condattr_t cattr; - eth_llc_data.fd_to_ef = malloc(sizeof(struct ef) * SYS_MAX_FLOWS); + eth_llc_data.fd_to_ef = + malloc(sizeof(*eth_llc_data.fd_to_ef) * SYS_MAX_FLOWS); if (eth_llc_data.fd_to_ef == NULL) goto fail_fd_to_ef; - eth_llc_data.ef_to_fd = malloc(sizeof(struct ef) * MAX_SAPS); + eth_llc_data.ef_to_fd = + malloc(sizeof(*eth_llc_data.ef_to_fd) * MAX_SAPS); if (eth_llc_data.ef_to_fd == NULL) goto fail_ef_to_fd; diff --git a/src/ipcpd/shim-udp/main.c b/src/ipcpd/shim-udp/main.c index 0bf57741..b6f516bd 100644 --- a/src/ipcpd/shim-udp/main.c +++ b/src/ipcpd/shim-udp/main.c @@ -693,10 +693,8 @@ static uint32_t ddns_resolve(char * name, char * addr_str = "Address:"; uint32_t ip_addr = 0; - if (inet_ntop(AF_INET, &dns_addr, dnsstr, INET_ADDRSTRLEN) == NULL) { + if (inet_ntop(AF_INET, &dns_addr, dnsstr, INET_ADDRSTRLEN) == NULL) return 0; - } - if (pipe(pipe_fd)) { log_err("Failed to create pipe."); @@ -743,7 +741,7 @@ static uint32_t ddns_resolve(char * name, substr = strtok(NULL, "\n"); } - if (strstr(substr2, addr_str) == NULL) { + if (substr2 == NULL || strstr(substr2, addr_str) == NULL) { log_err("Failed to resolve DNS address."); return 0; } diff --git a/src/irmd/main.c b/src/irmd/main.c index cc15e092..09a76214 100644 --- a/src/irmd/main.c +++ b/src/irmd/main.c @@ -307,6 +307,7 @@ static pid_t create_ipcp(char * name, entry = get_ipcp_entry_by_name(name); if (entry != NULL) { pthread_rwlock_unlock(&irmd.reg_lock); + free(api); log_err("IPCP by that name already exists."); return -1; } @@ -314,6 +315,7 @@ static pid_t create_ipcp(char * name, api->pid = ipcp_create(name, ipcp_type); if (api->pid == -1) { pthread_rwlock_unlock(&irmd.reg_lock); + free(api); log_err("Failed to create IPCP."); return -1; } @@ -321,6 +323,7 @@ static pid_t create_ipcp(char * name, tmp = ipcp_entry_create(); if (tmp == NULL) { pthread_rwlock_unlock(&irmd.reg_lock); + free(api); return -1; } @@ -330,6 +333,7 @@ static pid_t create_ipcp(char * name, if (tmp->name == NULL) { ipcp_entry_destroy(tmp); pthread_rwlock_unlock(&irmd.reg_lock); + free(api); return -1; } @@ -833,6 +837,11 @@ static ssize_t list_ipcps(char * name, count++; } + if (count == 0) { + pthread_rwlock_unlock(&irmd.reg_lock); + return 0; + } + *apis = malloc(count * sizeof(pid_t)); if (*apis == NULL) { pthread_rwlock_unlock(&irmd.reg_lock); @@ -1039,10 +1048,12 @@ static int api_announce(pid_t api, pthread_rwlock_unlock(&irmd.reg_lock); return -ENOMEM; } + n->str = strdup(s->str); if (n->str == NULL) { pthread_rwlock_unlock(&irmd.reg_lock); free(n); + return -ENOMEM; } list_add(&n->next, &e->names); diff --git a/src/irmd/registry.c b/src/irmd/registry.c index 3cc9b5f5..c7e7b52d 100644 --- a/src/irmd/registry.c +++ b/src/irmd/registry.c @@ -60,13 +60,13 @@ static struct reg_entry * reg_entry_create(void) return e; } -static struct reg_entry * reg_entry_init(struct reg_entry * e, - char * name) +static int reg_entry_init(struct reg_entry * e, + char * name) { pthread_condattr_t cattr; if (e == NULL || name == NULL) - return NULL; + return -1; list_head_init(&e->next); list_head_init(&e->difs); @@ -76,20 +76,20 @@ static struct reg_entry * reg_entry_init(struct reg_entry * e, e->name = name; if (pthread_condattr_init(&cattr)) - return NULL; + return -1; #ifndef __APPLE__ pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK); #endif if (pthread_cond_init(&e->state_cond, &cattr)) - return NULL; + return -1; if (pthread_mutex_init(&e->state_lock, NULL)) - return NULL; + return -1; e->state = REG_NAME_IDLE; - return e; + return 0; } static void reg_entry_destroy(struct reg_entry * e) @@ -166,17 +166,26 @@ static int reg_entry_add_local_in_dif(struct reg_entry * e, const char * dif_name, enum ipcp_type type) { - if (!reg_entry_is_local_in_dif(e, dif_name)) { - struct reg_dif * rdn = malloc(sizeof(*rdn)); - rdn->dif_name = strdup(dif_name); - if (rdn->dif_name == NULL) - return -1; - rdn->type = type; - list_add(&rdn->next, &e->difs); + struct reg_dif * rdn; + + /* already registered. Is ok */ + if (reg_entry_is_local_in_dif(e, dif_name)) return 0; + + rdn = malloc(sizeof(*rdn)); + if (rdn == NULL) + return -1; + + rdn->dif_name = strdup(dif_name); + if (rdn->dif_name == NULL) { + free(rdn); + return -1; } - return 0; /* already registered. Is ok */ + rdn->type = type; + list_add(&rdn->next, &e->difs); + + return 0; } static void reg_entry_del_local_from_dif(struct reg_entry * e, @@ -230,8 +239,10 @@ int reg_entry_add_apn(struct reg_entry * e, return -ENOMEM; n->str = strdup(a->apn); - if (n->str == NULL) + if (n->str == NULL) { + free(n); return -ENOMEM; + } list_add(&n->next, &e->reg_apns); @@ -546,6 +557,8 @@ struct reg_entry * registry_get_entry_by_hash(struct list_head * registry, } } + free(thash); + return NULL; } @@ -568,10 +581,9 @@ struct reg_entry * registry_add_name(struct list_head * registry, return NULL; } - e = reg_entry_init(e, strdup(name)); - if (e == NULL) { - log_dbg("Could not initialize registry entry."); + if (reg_entry_init(e, strdup(name))) { reg_entry_destroy(e); + log_dbg("Could not initialize registry entry."); return NULL; } diff --git a/src/lib/btree.c b/src/lib/btree.c index 8be8b99f..beba90f7 100644 --- a/src/lib/btree.c +++ b/src/lib/btree.c @@ -85,6 +85,8 @@ static struct btnode * btnode_create(size_t k) if (node == NULL) return NULL; + assert(k > 0); + node->keyvals = malloc(sizeof(*node->keyvals) * k); if (node->keyvals == NULL) { free(node); @@ -368,8 +370,10 @@ struct btree * btree_create(size_t k) if (tree == NULL) return NULL; - if (k > BTREE_MAX_ORDER) + if (k < 1 || k > BTREE_MAX_ORDER) { + free(tree); return NULL; + } tree->k = k; tree->root = NULL; @@ -413,8 +417,10 @@ int btree_insert(struct btree * tree, if (rgt != NULL) { struct btnode * lft = btnode_create(tree->root->k); - if (lft == NULL) + if (lft == NULL) { + free(rgt); return -ENOMEM; + } lft->used = tree->root->used; lft->leaf = tree->root->leaf; diff --git a/src/lib/cdap.c b/src/lib/cdap.c index 679771f5..5ed86ad1 100644 --- a/src/lib/cdap.c +++ b/src/lib/cdap.c @@ -555,6 +555,7 @@ int cdap_add_flow(struct cdap * instance, if (flow_set_add(instance->set, fd)) { pthread_rwlock_unlock(&instance->flows_lock); + free(e); return -1; } diff --git a/src/lib/frct_pci.c b/src/lib/frct_pci.c index 115a3eb9..4fa9ddc2 100644 --- a/src/lib/frct_pci.c +++ b/src/lib/frct_pci.c @@ -71,7 +71,7 @@ int frct_pci_ser(struct shm_du_buff * sdb, if (pci->type & PDU_TYPE_CONFIG) { memcpy(head + offset, &pci->conf_flags, CONF_FLAGS_SIZE); - offset += CONF_FLAGS_SIZE; + /* offset += CONF_FLAGS_SIZE; */ } if (error_check) { @@ -113,7 +113,7 @@ int frct_pci_des(struct shm_du_buff * sdb, if (pci->type & PDU_TYPE_CONFIG) { memcpy(&pci->conf_flags, head + offset, CONF_FLAGS_SIZE); - offset += CONF_FLAGS_SIZE; + /* offset += CONF_FLAGS_SIZE; */ } if (error_check) { diff --git a/src/lib/irm.c b/src/lib/irm.c index ce17bf18..7e593cc6 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -333,7 +333,6 @@ static int check_ap_path(char ** ap_name) strcpy(tstop--, *ap_name); while (pstop < path_end) { - ret = 0; pstart = pstop; if (*pstart != '/') { free(tmp); diff --git a/src/lib/rib.c b/src/lib/rib.c index bbe996e7..9e45a302 100644 --- a/src/lib/rib.c +++ b/src/lib/rib.c @@ -788,7 +788,7 @@ static struct rib_sub * rib_get_sub(uint32_t sid) return r; } - return 0; + return NULL; } static struct rib_sub * rib_sub_create(uint32_t sid) @@ -1140,6 +1140,8 @@ int rib_event_wait(ro_set_t * set, sub = rib_get_sub(set->sid); + assert(sub); + pthread_rwlock_unlock(&rib.lock); pthread_mutex_lock(&sub->lock); diff --git a/src/lib/shm_rdrbuff.c b/src/lib/shm_rdrbuff.c index 447f8b35..d8893677 100644 --- a/src/lib/shm_rdrbuff.c +++ b/src/lib/shm_rdrbuff.c @@ -147,8 +147,10 @@ struct shm_rdrbuff * shm_rdrbuff_create() return NULL; rdrb = malloc(sizeof *rdrb); - if (rdrb == NULL) + if (rdrb == NULL) { + free(shm_rdrb_fn); return NULL; + } mask = umask(0); @@ -227,8 +229,10 @@ struct shm_rdrbuff * shm_rdrbuff_open() return NULL; rdrb = malloc(sizeof *rdrb); - if (rdrb == NULL) + if (rdrb == NULL) { + free(shm_rdrb_fn); return NULL; + } shm_fd = shm_open(shm_rdrb_fn, O_RDWR, 0666); if (shm_fd < 0) { -- cgit v1.2.3