From 6594b2b8842f92bd89070a5c86adcfb67fdcaac9 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 29 Mar 2017 11:00:50 -0400 Subject: [PATCH] fixes for coverity Change-Id: I92eebf9a44fed8d826e0c2a207c05cd02c5ade0c --- Makefile | 39 +++++++++++++++++++++++++++++-------- daemon/call.c | 4 ++-- daemon/cli.c | 29 ++++++++++++++------------- daemon/ice.c | 1 + daemon/main.c | 8 ++++---- daemon/media_socket.c | 3 ++- daemon/poller.c | 1 + daemon/recording.c | 2 ++ daemon/redis.c | 5 +++++ daemon/socket.c | 6 ++++-- kernel-module/Makefile | 1 + lib/lib.Makefile | 6 +++++- recording-daemon/metafile.c | 8 ++++++-- 13 files changed, 79 insertions(+), 34 deletions(-) diff --git a/Makefile b/Makefile index 5e92a1cfb..d17afc873 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,34 @@ +RTPENGINE_ROOT_DIR=. + +include lib/lib.Makefile + +.PHONY: all clean coverity + all: - make -C daemon - make -C recording-daemon - make -C iptables-extension - make -C kernel-module + $(MAKE) -C daemon + $(MAKE) -C recording-daemon + $(MAKE) -C iptables-extension + $(MAKE) -C kernel-module + +clean: + $(MAKE) -C daemon clean + $(MAKE) -C recording-daemon clean + $(MAKE) -C iptables-extension clean + $(MAKE) -C kernel-module clean + rm -rf project.tgz cov-int .DEFAULT: - make -C daemon $@ - make -C recording-daemon $@ - make -C iptables-extension $@ - make -C kernel-module $@ + $(MAKE) -C daemon $@ + $(MAKE) -C recording-daemon $@ + $(MAKE) -C iptables-extension $@ + $(MAKE) -C kernel-module $@ + +coverity: + cov-build --dir cov-int $(MAKE) + tar -czf project.tgz cov-int + curl --form token=$(COVERITY_RTPENGINE_TOKEN) \ + --form email=$(DEBEMAIL) \ + --form file=@project.tgz \ + --form version="$(RTPENGINE_VERSION)" \ + --form description="automatic upload" \ + https://scan.coverity.com/builds?project=$(COVERITY_RTPENGINE_PROJECT) diff --git a/daemon/call.c b/daemon/call.c index 3711beaa3..a49c946ce 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -622,8 +622,8 @@ struct callmaster *callmaster_new(struct poller *p) { mutex_init(&c->totalstats_interval.total_calls_duration_lock); c->totalstats.started = poller_now; - c->totalstats_interval.managed_sess_min = 0; - c->totalstats_interval.managed_sess_max = 0; + //c->totalstats_interval.managed_sess_min = 0; // already zeroed + //c->totalstats_interval.managed_sess_max = 0; mutex_init(&c->totalstats_lastinterval_lock); mutex_init(&c->cngs_lock); diff --git a/daemon/cli.c b/daemon/cli.c index eab242b70..a0af6fd9b 100644 --- a/daemon/cli.c +++ b/daemon/cli.c @@ -463,7 +463,7 @@ static void cli_incoming_set_maxopenfiles(char* buffer, int len, struct callmast open_files.len = len; open_files_num = strtoul(open_files.s, &endptr, 10); - if ((errno == ERANGE && (open_files_num == LONG_MAX || open_files_num == LONG_MIN)) || (errno != 0 && open_files_num == 0)) { + if ((errno == ERANGE && (open_files_num == ULONG_MAX)) || (errno != 0 && open_files_num == 0)) { printlen = snprintf (replybuffer,(outbufend-replybuffer), "Fail setting open_files to %.*s; errno=%d\n", open_files.len, open_files.s, errno); ADJUSTLEN(printlen,outbufend,replybuffer); return; @@ -549,7 +549,7 @@ static void cli_incoming_set_timeout(char* buffer, int len, struct callmaster* m timeout.len = len; timeout_num = strtoul(timeout.s, &endptr, 10); - if ((errno == ERANGE && (timeout_num == LONG_MAX || timeout_num == LONG_MIN)) || (errno != 0 && timeout_num == 0)) { + if ((errno == ERANGE && (timeout_num == ULONG_MAX)) || (errno != 0 && timeout_num == 0)) { printlen = snprintf (replybuffer,(outbufend-replybuffer), "Fail setting timeout to %.*s; errno=%d\n", timeout.len, timeout.s, errno); ADJUSTLEN(printlen,outbufend,replybuffer); return; @@ -725,7 +725,7 @@ static void cli_incoming_terminate(char* buffer, int len, struct callmaster* m, static void cli_incoming_ksadd(char* buffer, int len, struct callmaster* m, char* replybuffer, const char* outbufend) { int printlen=0; - unsigned int uint_keyspace_db; + unsigned long uint_keyspace_db; str str_keyspace_db; char *endptr; @@ -738,9 +738,9 @@ static void cli_incoming_ksadd(char* buffer, int len, struct callmaster* m, char str_keyspace_db.s = buffer; str_keyspace_db.len = len; - uint_keyspace_db = strtol(str_keyspace_db.s, &endptr, 10); + uint_keyspace_db = strtoul(str_keyspace_db.s, &endptr, 10); - if ((errno == ERANGE && (uint_keyspace_db == LONG_MAX || uint_keyspace_db == LONG_MIN)) || (errno != 0 && uint_keyspace_db == 0)) { + if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || (errno != 0 && uint_keyspace_db == 0)) { printlen = snprintf(replybuffer, outbufend-replybuffer, "Fail adding keyspace %.*s to redis notifications; errono=%d\n", str_keyspace_db.len, str_keyspace_db.s, errno); } else if (endptr == str_keyspace_db.s) { printlen = snprintf(replybuffer, outbufend-replybuffer, "Fail adding keyspace %.*s to redis notifications; no digists found\n", str_keyspace_db.len, str_keyspace_db.s); @@ -749,9 +749,9 @@ static void cli_incoming_ksadd(char* buffer, int len, struct callmaster* m, char if (!g_queue_find(m->conf.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db))) { g_queue_push_tail(m->conf.redis_subscribed_keyspaces, GUINT_TO_POINTER(uint_keyspace_db)); redis_notify_subscribe_action(m, SUBSCRIBE_KEYSPACE, uint_keyspace_db); - printlen = snprintf(replybuffer, outbufend-replybuffer, "Success adding keyspace %u to redis notifications.\n", uint_keyspace_db); + printlen = snprintf(replybuffer, outbufend-replybuffer, "Success adding keyspace %lu to redis notifications.\n", uint_keyspace_db); } else { - printlen = snprintf(replybuffer, outbufend-replybuffer, "Keyspace %u is already among redis notifications.\n", uint_keyspace_db); + printlen = snprintf(replybuffer, outbufend-replybuffer, "Keyspace %lu is already among redis notifications.\n", uint_keyspace_db); } rwlock_unlock_w(&m->conf.config_lock); } @@ -761,7 +761,7 @@ static void cli_incoming_ksadd(char* buffer, int len, struct callmaster* m, char static void cli_incoming_ksrm(char* buffer, int len, struct callmaster* m, char* replybuffer, const char* outbufend) { int printlen = 0; GList *l; - unsigned int uint_keyspace_db; + unsigned long uint_keyspace_db; str str_keyspace_db; char *endptr; @@ -774,10 +774,10 @@ static void cli_incoming_ksrm(char* buffer, int len, struct callmaster* m, char* str_keyspace_db.s = buffer; str_keyspace_db.len = len; - uint_keyspace_db = strtol(str_keyspace_db.s, &endptr, 10); + uint_keyspace_db = strtoul(str_keyspace_db.s, &endptr, 10); rwlock_lock_w(&m->conf.config_lock); - if ((errno == ERANGE && (uint_keyspace_db == LONG_MAX || uint_keyspace_db == LONG_MIN)) || (errno != 0 && uint_keyspace_db == 0)) { + if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || (errno != 0 && uint_keyspace_db == 0)) { printlen = snprintf(replybuffer, outbufend-replybuffer, "Fail removing keyspace %.*s to redis notifications; errono=%d\n", str_keyspace_db.len, str_keyspace_db.s, errno); } else if (endptr == str_keyspace_db.s) { printlen = snprintf(replybuffer, outbufend-replybuffer, "Fail removing keyspace %.*s to redis notifications; no digists found\n", str_keyspace_db.len, str_keyspace_db.s); @@ -785,15 +785,15 @@ static void cli_incoming_ksrm(char* buffer, int len, struct callmaster* m, char* // remove this keyspace redis_notify_subscribe_action(m, UNSUBSCRIBE_KEYSPACE, uint_keyspace_db); g_queue_remove(m->conf.redis_subscribed_keyspaces, l->data); - printlen = snprintf(replybuffer, outbufend-replybuffer, "Successfully unsubscribed from keyspace %u.\n", uint_keyspace_db); + printlen = snprintf(replybuffer, outbufend-replybuffer, "Successfully unsubscribed from keyspace %lu.\n", uint_keyspace_db); // destroy foreign calls for this keyspace destroy_keyspace_foreign_calls(m, uint_keyspace_db); // update cli - printlen = snprintf(replybuffer, outbufend-replybuffer, "Successfully removed all foreign calls for keyspace %u.\n", uint_keyspace_db); + printlen = snprintf(replybuffer, outbufend-replybuffer, "Successfully removed all foreign calls for keyspace %lu.\n", uint_keyspace_db); } else { - printlen = snprintf(replybuffer, outbufend-replybuffer, "Keyspace %u is not among redis notifications.\n", uint_keyspace_db); + printlen = snprintf(replybuffer, outbufend-replybuffer, "Keyspace %lu is not among redis notifications.\n", uint_keyspace_db); } rwlock_unlock_w(&m->conf.config_lock); @@ -841,7 +841,7 @@ next: if (nfd == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) { ilog(LOG_INFO, "Could currently not accept CLI commands. Reason:%s", strerror(errno)); - goto cleanup; + goto cleanup2; } ilog(LOG_INFO, "Accept error:%s", strerror(errno)); goto next; @@ -893,6 +893,7 @@ next: cleanup: close(nfd); +cleanup2: mutex_unlock(&cli->lock); log_info_clear(); } diff --git a/daemon/ice.c b/daemon/ice.c index b39ed3643..bd872a924 100644 --- a/daemon/ice.c +++ b/daemon/ice.c @@ -1135,6 +1135,7 @@ int ice_request(struct stream_fd *sfd, const endpoint_t *src, mutex_lock(&ag->lock); + // coverity[use : FALSE] g_tree_insert(ag->nominated_pairs, pair, pair); if (PAIR_ISSET(pair, SUCCEEDED)) { diff --git a/daemon/main.c b/daemon/main.c index 6b8ee4c8f..140dd222c 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -256,7 +256,7 @@ static int redis_ep_parse(endpoint_t *ep, int *db, char **auth, const char *auth static void options(int *argc, char ***argv) { char **if_a = NULL; char **ks_a = NULL; - unsigned int uint_keyspace_db; + unsigned long uint_keyspace_db; str str_keyspace_db; char **iter; struct intf_config *ifa; @@ -338,13 +338,13 @@ static void options(int *argc, char ***argv) { for (iter = ks_a; *iter; iter++) { str_keyspace_db.s = *iter; str_keyspace_db.len = strlen(*iter); - uint_keyspace_db = strtol(str_keyspace_db.s, &endptr, 10); + uint_keyspace_db = strtoul(str_keyspace_db.s, &endptr, 10); - if ((errno == ERANGE && (uint_keyspace_db == LONG_MAX || uint_keyspace_db == LONG_MIN)) || + if ((errno == ERANGE && (uint_keyspace_db == ULONG_MAX)) || (errno != 0 && uint_keyspace_db == 0)) { ilog(LOG_ERR, "Fail adding keyspace %.*s to redis notifications; errono=%d\n", str_keyspace_db.len, str_keyspace_db.s, errno); } else if (endptr == str_keyspace_db.s) { - ilog(LOG_ERR, "Fail adding keyspace %.*s to redis notifications; no digists found\n", str_keyspace_db.len, str_keyspace_db.s); + ilog(LOG_ERR, "Fail adding keyspace %.*s to redis notifications; no digits found\n", str_keyspace_db.len, str_keyspace_db.s); } else { g_queue_push_tail(&keyspaces, GUINT_TO_POINTER(uint_keyspace_db)); } diff --git a/daemon/media_socket.c b/daemon/media_socket.c index 6a6e4e9df..4fe46718b 100644 --- a/daemon/media_socket.c +++ b/daemon/media_socket.c @@ -1516,7 +1516,8 @@ struct stream_fd *stream_fd_new(socket_t *fd, struct call *call, const struct lo pi.readable = stream_fd_readable; pi.closed = stream_fd_closed; - poller_add_item(po, &pi); + if (poller_add_item(po, &pi)) + ilog(LOG_ERR, "Failed to add stream_fd to poller"); return sfd; } diff --git a/daemon/poller.c b/daemon/poller.c index 9b0bbc694..9382acf91 100644 --- a/daemon/poller.c +++ b/daemon/poller.c @@ -457,6 +457,7 @@ static int poller_timer_link(struct poller *p, GSList **lp, void (*f)(void *), s mutex_lock(&p->timers_add_del_lock); *lp = g_slist_prepend(*lp, i); + // coverity[lock_order : FALSE] if (!mutex_trylock(&p->timers_lock)) { poller_timers_mod(p); mutex_unlock(&p->timers_lock); diff --git a/daemon/recording.c b/daemon/recording.c index 7c491e476..8427d531c 100644 --- a/daemon/recording.c +++ b/daemon/recording.c @@ -162,6 +162,7 @@ static int check_create_dir(const char *dir, const char *desc, mode_t creat_mode return FALSE; } ilog(LOG_INFO, "Creating %s directory \"%s\".", desc, dir); + // coverity[toctou : FALSE] if (mkdir(dir, creat_mode) == 0) return TRUE; ilog(LOG_ERR, "Failed to create %s directory \"%s\": %s", desc, dir, strerror(errno)); @@ -317,6 +318,7 @@ static char *meta_setup_file(struct recording *recording) { char *meta_filepath = file_path_str(recording->meta_prefix, "/tmp/rtpengine-meta-", ".tmp"); recording->meta_filepath = meta_filepath; FILE *mfp = fopen(meta_filepath, "w"); + // coverity[check_return : FALSE] chmod(meta_filepath, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH); if (mfp == NULL) { ilog(LOG_ERROR, "Could not open metadata file: %s", meta_filepath); diff --git a/daemon/redis.c b/daemon/redis.c index fde2ff9e3..a15d0ad67 100644 --- a/daemon/redis.c +++ b/daemon/redis.c @@ -998,6 +998,7 @@ static int redis_hash_get_crypto_context(struct crypto_context *out, const struc if (redis_hash_get_u64(&out->last_index, h, "last_index")) return -1; + // coverity[check_return : FALSE] redis_hash_get_unsigned(&out->ssrc, h, "ssrc"); return 0; @@ -1574,6 +1575,7 @@ int redis_restore(struct callmaster *m, struct redis *r) { rlog(LOG_DEBUG, "Restoring calls from Redis..."); mutex_lock(&r->lock); + // coverity[sleep : FALSE] if (redis_check_conn(r) == REDIS_STATE_DISCONNECTED) { mutex_unlock(&r->lock); ret = 0; @@ -2001,6 +2003,7 @@ void redis_update_onekey(struct call *c, struct redis *r) { return; mutex_lock(&r->lock); + // coverity[sleep : FALSE] if (redis_check_conn(r) == REDIS_STATE_DISCONNECTED) { mutex_unlock(&r->lock); return ; @@ -2048,6 +2051,7 @@ void redis_delete(struct call *c, struct redis *r) { return; mutex_lock(&r->lock); + // coverity[sleep : FALSE] if (redis_check_conn(r) == REDIS_STATE_DISCONNECTED) { mutex_unlock(&r->lock); return ; @@ -2082,6 +2086,7 @@ void redis_wipe(struct redis *r) { return; mutex_lock(&r->lock); + // coverity[sleep : FALSE] if (redis_check_conn(r) == REDIS_STATE_DISCONNECTED) { mutex_unlock(&r->lock); return ; diff --git a/daemon/socket.c b/daemon/socket.c index 6e5b4c7ef..9a1bc4f16 100644 --- a/daemon/socket.c +++ b/daemon/socket.c @@ -324,11 +324,13 @@ static ssize_t __ip_sendto(socket_t *s, const void *buf, size_t len, const endpo static int __ip4_tos(socket_t *s, unsigned int tos) { unsigned char ctos; ctos = tos; - setsockopt(s->fd, IPPROTO_IP, IP_TOS, &ctos, sizeof(ctos)); + if (setsockopt(s->fd, IPPROTO_IP, IP_TOS, &ctos, sizeof(ctos))) + ilog(LOG_ERR, "Failed to set TOS on IPv4 socket: %s", strerror(errno)); return 0; } static int __ip6_tos(socket_t *s, unsigned int tos) { - setsockopt(s->fd, IPPROTO_IPV6, IPV6_TCLASS, &tos, sizeof(tos)); + if (setsockopt(s->fd, IPPROTO_IPV6, IPV6_TCLASS, &tos, sizeof(tos))) + ilog(LOG_ERR, "Failed to set TOS on IPv6 socket: %s", strerror(errno)); return 0; } static int __ip_error(socket_t *s) { diff --git a/kernel-module/Makefile b/kernel-module/Makefile index 6900cd607..23e7a6cc1 100644 --- a/kernel-module/Makefile +++ b/kernel-module/Makefile @@ -1,6 +1,7 @@ PWD := $(shell pwd) KSRC ?= /lib/modules/$(shell uname -r)/build KBUILD := $(KSRC) +M ?= $(PWD) ifeq ($(RTPENGINE_VERSION),) DPKG_PRSCHNGLG= $(shell which dpkg-parsechangelog 2>/dev/null) diff --git a/lib/lib.Makefile b/lib/lib.Makefile index 7fe7de986..d2867fb4b 100644 --- a/lib/lib.Makefile +++ b/lib/lib.Makefile @@ -1,10 +1,14 @@ CC ?= gcc +ifeq ($(RTPENGINE_ROOT_DIR),) + RTPENGINE_ROOT_DIR=.. +endif + ifeq ($(RTPENGINE_VERSION),) DPKG_PRSCHNGLG= $(shell which dpkg-parsechangelog 2>/dev/null) ifneq ($(DPKG_PRSCHNGLG),) - DPKG_PRSCHNGLG=$(shell dpkg-parsechangelog -l../debian/changelog | awk '/^Version: / {print $$2}') + DPKG_PRSCHNGLG=$(shell dpkg-parsechangelog -l$(RTPENGINE_ROOT_DIR)/debian/changelog | awk '/^Version: / {print $$2}') endif GIT_BR_COMMIT=$(shell git branch --no-color --no-column -v 2> /dev/null | awk '/^\*/ {OFS="-"; print "git", $$2, $$3}') diff --git a/recording-daemon/metafile.c b/recording-daemon/metafile.c index 2d21892fa..6d59400dc 100644 --- a/recording-daemon/metafile.c +++ b/recording-daemon/metafile.c @@ -159,10 +159,14 @@ void metafile_change(char *name) { // open file and seek to last known position int fd = open(fnbuf, O_RDONLY); if (fd == -1) { - ilog(LOG_ERR, "Failed to open %s: %s\n", fnbuf, strerror(errno)); + ilog(LOG_ERR, "Failed to open %s: %s", fnbuf, strerror(errno)); + goto out; + } + if (lseek(fd, mf->pos, SEEK_SET) == (off_t) -1) { + ilog(LOG_ERR, "Failed to seek to end of file %s: %s", fnbuf, strerror(errno)); + close(fd); goto out; } - lseek(fd, mf->pos, SEEK_SET); // read the entire file GString *s = g_string_new(NULL);