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 <[email protected]>
Signed-off-by: Dimitri Staessens <[email protected]>

6 files changed, 58 insertions, 31 deletions

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)