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 <sander.vrijders@xxxxxxxx>
---
src/lib/cacep.c | 8 +++++++-
src/lib/frct.c | 2 +-
src/lib/irm.c | 10 +++++++---
src/lib/rib.c | 15 +++++++++++++-
src/lib/shm_flow_set.c | 44 ++++++++++++++++++++++++------------------
src/lib/timerwheel.c | 3 ---
6 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/src/lib/cacep.c b/src/lib/cacep.c
index 6efb729..62c4869 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);
@@ -73,6 +78,7 @@ static int send_msg(int fd,
msg.address = info->addr;
msg.pref_version = info->pref_version;
msg.pref_syntax = info->pref_syntax;
+
if (msg.pref_syntax < 0)
return -1;
diff --git a/src/lib/frct.c b/src/lib/frct.c
index 296d5b2..404e1dc 100644
--- a/src/lib/frct.c
+++ b/src/lib/frct.c
@@ -406,7 +406,7 @@ static int __frcti_rcv(struct frcti * frcti,
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 bd34669..66243c3 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,13 @@ static int check_prog_path(char ** prog)
assert(prog);
+ path = getenv("PATH");
if (*prog == NULL || path == NULL)
return -EINVAL;
+ 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 685575e..88db9ed 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 bb9e3ca..008e4a0 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 ef8489b..0feda64 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)
--
2.19.0
