From 937f2b345aa76272a1c80828e7666ab87611c0d1 Mon Sep 17 00:00:00 2001 From: Sander Vrijders Date: Thu, 27 Sep 2018 11:43:02 +0200 Subject: lib: Check return values init functions This will check the return values of init functions so that the code is more robust. It also removes a duplicate init in the timerwheel, checks for buffer overflows in the RIB and checks string lengths. Signed-off-by: Sander Vrijders Signed-off-by: Dimitri Staessens --- src/lib/cacep.c | 7 ++++++- src/lib/frct.c | 6 +++--- src/lib/irm.c | 14 ++++++++++---- src/lib/rib.c | 15 ++++++++++++++- src/lib/shm_flow_set.c | 44 +++++++++++++++++++++++++------------------- src/lib/timerwheel.c | 3 --- 6 files changed, 58 insertions(+), 31 deletions(-) (limited to 'src/lib') diff --git a/src/lib/cacep.c b/src/lib/cacep.c index 6efb7295..12751078 100644 --- a/src/lib/cacep.c +++ b/src/lib/cacep.c @@ -32,7 +32,7 @@ #include "cacep.pb-c.h" typedef CacepMsg cacep_msg_t; -#define BUF_SIZE 64 +#define BUF_SIZE 128 static int read_msg(int fd, struct conn_info * info) @@ -49,6 +49,11 @@ static int read_msg(int fd, if (msg == NULL) return -1; + if (strlen(msg->comp_name) > CACEP_BUF_STRLEN) { + cacep_msg__free_unpacked(msg, NULL); + return -1; + } + strcpy(info->comp_name, msg->comp_name); strcpy(info->protocol, msg->protocol); diff --git a/src/lib/frct.c b/src/lib/frct.c index c057ba50..516c958b 100644 --- a/src/lib/frct.c +++ b/src/lib/frct.c @@ -357,15 +357,15 @@ static int __frcti_rcv(struct frcti * frcti, rcv_cr->act = now.tv_sec; + pthread_rwlock_unlock(&frcti->lock); + if (!(pci->flags & FRCT_DATA)) shm_rdrbuff_remove(ai.rdrb, idx); - pthread_rwlock_unlock(&frcti->lock); - return ret; drop_packet: - shm_rdrbuff_remove(ai.rdrb, idx); pthread_rwlock_unlock(&frcti->lock); + shm_rdrbuff_remove(ai.rdrb, idx); return -EAGAIN; } diff --git a/src/lib/irm.c b/src/lib/irm.c index bd34669f..d88475c4 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -319,10 +319,10 @@ static int check_prog(const char * prog) static int check_prog_path(char ** prog) { - char * path = getenv("PATH"); - char * path_end = path + strlen(path) + 1; + char * path; + char * path_end; char * pstart; - char * pstop = path; + char * pstop; char * tmp; char * tstop; char * tstart; @@ -331,9 +331,15 @@ static int check_prog_path(char ** prog) assert(prog); - if (*prog == NULL || path == NULL) + if (*prog == NULL) return -EINVAL; + path = getenv("PATH"); + if (path == NULL) + return -ENOENT; + + pstop = path; + path_end = path + strlen(path) + 1; if (!strlen(path) || strchr(*prog, '/') != NULL) { if ((ret = check_prog(*prog)) < 0) return ret; diff --git a/src/lib/rib.c b/src/lib/rib.c index 685575e5..88db9ed8 100644 --- a/src/lib/rib.c +++ b/src/lib/rib.c @@ -101,6 +101,9 @@ static int rib_read(const char * path, char comp[RIB_PATH_LEN + 1]; char * c; + if (strlen(path) > RIB_PATH_LEN) + return -1; + strcpy(comp, path + 1); c = strstr(comp, "/"); @@ -183,6 +186,9 @@ static size_t __getattr(const char * path, char comp[RIB_PATH_LEN + 1]; char * c; + if (strlen(path) > RIB_PATH_LEN) + return -1; + strcpy(comp, path + 1); c = strstr(comp, "/"); @@ -282,7 +288,8 @@ int rib_init(const char * mountpt) if (stat(rib.mnt, &st) == -1) switch(errno) { case ENOENT: - mkdir(rib.mnt, 0777); + if (mkdir(rib.mnt, 0777)) + return -1; break; case ENOTCONN: fuse_unmount(rib.mnt, rib.ch); @@ -385,6 +392,12 @@ int rib_reg(const char * path, return -ENOMEM; } + if (strlen(path) > RIB_PATH_LEN) { + pthread_rwlock_unlock(&rib.lock); + free(rc); + return -1; + } + strcpy(rc->path, path); rc->ops = ops; diff --git a/src/lib/shm_flow_set.c b/src/lib/shm_flow_set.c index bb9e3caa..008e4a0d 100644 --- a/src/lib/shm_flow_set.c +++ b/src/lib/shm_flow_set.c @@ -98,17 +98,14 @@ struct shm_flow_set * shm_flow_set_create() mask = umask(0); shm_fd = shm_open(fn, O_CREAT | O_RDWR, 0666); - if (shm_fd == -1) { - free(set); - return NULL; - } + if (shm_fd == -1) + goto fail_shm_open; umask(mask); if (ftruncate(shm_fd, SHM_FLOW_SET_FILE_SIZE - 1) < 0) { - free(set); close(shm_fd); - return NULL; + goto fail_shm_open; } shm_base = mmap(NULL, @@ -120,11 +117,8 @@ struct shm_flow_set * shm_flow_set_create() close(shm_fd); - if (shm_base == MAP_FAILED) { - shm_unlink(fn); - free(set); - return NULL; - } + if (shm_base == MAP_FAILED) + goto fail_mmap; set->mtable = shm_base; set->heads = (size_t *) (set->mtable + SYS_MAX_FLOWS); @@ -133,21 +127,27 @@ struct shm_flow_set * shm_flow_set_create() set->lock = (pthread_mutex_t *) (set->fqueues + PROG_MAX_FQUEUES * (SHM_BUFFER_SIZE)); - pthread_mutexattr_init(&mattr); + if (pthread_mutexattr_init(&mattr)) + goto fail_mmap; + #ifdef HAVE_ROBUST_MUTEX - pthread_mutexattr_setrobust(&mattr, PTHREAD_MUTEX_ROBUST); + if (pthread_mutexattr_setrobust(&mattr, PTHREAD_MUTEX_ROBUST)) + goto fail_mmap; #endif - pthread_mutexattr_setpshared(&mattr, PTHREAD_PROCESS_SHARED); - pthread_mutex_init(set->lock, &mattr); + if (pthread_mutexattr_setpshared(&mattr, PTHREAD_PROCESS_SHARED) || + pthread_mutex_init(set->lock, &mattr) || + pthread_condattr_init(&cattr) || + pthread_condattr_setpshared(&cattr, PTHREAD_PROCESS_SHARED)) + goto fail_mmap; - pthread_condattr_init(&cattr); - pthread_condattr_setpshared(&cattr, PTHREAD_PROCESS_SHARED); #ifndef __APPLE__ - pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK); + if (pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK)) + goto fail_mmap; #endif for (i = 0; i < PROG_MAX_FQUEUES; ++i) { set->heads[i] = 0; - pthread_cond_init(&set->conds[i], &cattr); + if (pthread_cond_init(&set->conds[i], &cattr)) + goto fail_mmap; } for (i = 0; i < SYS_MAX_FLOWS; ++i) @@ -156,6 +156,12 @@ struct shm_flow_set * shm_flow_set_create() set->pid = getpid(); return set; + + fail_mmap: + shm_unlink(fn); + fail_shm_open: + free(set); + return NULL; } struct shm_flow_set * shm_flow_set_open(pid_t pid) diff --git a/src/lib/timerwheel.c b/src/lib/timerwheel.c index ef8489bf..0feda642 100644 --- a/src/lib/timerwheel.c +++ b/src/lib/timerwheel.c @@ -121,9 +121,6 @@ struct timerwheel * timerwheel_create(time_t resolution, if (tw == NULL) return NULL; - if (pthread_mutex_init(&tw->lock, NULL)) - return NULL; - tw->elements = 1; while (tw->elements < (size_t) max_delay / resolution) -- cgit v1.2.3