From 29224763a2df77d1c7e906a4a41d02544903c141 Mon Sep 17 00:00:00 2001
From: Michal Bloch <m.bloch@samsung.com>
Date: Thu, 26 Sep 2019 13:07:19 +0200
Subject: [PATCH] Fix some issues found by static analysis.

Change-Id: Icd9b6278f016b7bf73c7dd47892e3c2c395bae8d
Signed-off-by: Michal Bloch <m.bloch@samsung.com>
---
 src/config.c                      |  3 ++-
 src/process.c                     | 13 ++++++++++---
 src/raw_data_providers/proc_tsm.c |  9 +++------
 src/utils.c                       | 16 ++++++++++------
 src/utils.h                       |  2 +-
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/config.c b/src/config.c
index 3287b62..9f96041 100644
--- a/src/config.c
+++ b/src/config.c
@@ -80,7 +80,8 @@ static int is_regular_file(const char *path)
 {
     struct stat st;
 
-    stat(path, &st);
+    if (stat(path, &st) < 0)
+      return 0;
 
     return S_ISREG(st.st_mode);
 }
diff --git a/src/process.c b/src/process.c
index ce70727..212307e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -87,6 +87,7 @@ int process_create_info(int pid, int ppid, struct proc_info **process)
     p->data_sources = calloc(data_sources_get_n(), sizeof(struct data_source));
     if (!p->data_sources) {
         _E("Unable to allocate memory");
+        free(p);
         return -ENOMEM;
     }
 
@@ -101,6 +102,7 @@ int process_create_info(int pid, int ppid, struct proc_info **process)
         ret = ds_mod->init(ds);
         if (ret) {
             _E("Unable to initialize new %s data source: %d", ds_mod->name, ret);
+            process_cleanup(p);
             return ret;
         }
     }
@@ -149,20 +151,20 @@ static int process_get_appid(struct proc_info *process)
 
 static int process_get_exe(struct proc_info *process)
 {
-    char buf[NAME_MAX];
+    char buf[NAME_MAX + 1]; // +1 for null terminator
     int ret;
 
     free(process->exe);
     process->exe = NULL;
 
-    ret = pid_to_basename(process->pid, buf);
+    ret = pid_to_basename(process->pid, buf, sizeof(buf));
     if (ret) {
         if (ret != -EEXIST)
             _E("pid_to_basename() failed, pid: %d, ret: %d", process->pid, ret);
         return ret;
     }
 
-    process->exe = strndup(buf, NAME_MAX);
+    process->exe = strndup(buf, sizeof(buf));
     if (!process->exe) {
         _E("Unable to allocate memory");
         return -ENOMEM;
@@ -519,6 +521,11 @@ static void process_cleanup(struct proc_info *process)
     for (int i = 0; i < data_sources_get_n(); i++) {
         ds = &(process->data_sources[i]);
 
+        /* Uninitialized list elements can happen if the object
+         * failed halfway through its initialisation. */
+        if (!ds->data_fg.samples.next)
+            continue;
+
         /* Remove samples */
         list_for_each_entry_safe(sample, sample_next, &ds->data_fg.samples, node) {
             list_del(&sample->node);
diff --git a/src/raw_data_providers/proc_tsm.c b/src/raw_data_providers/proc_tsm.c
index 7656564..3c18cce 100644
--- a/src/raw_data_providers/proc_tsm.c
+++ b/src/raw_data_providers/proc_tsm.c
@@ -103,13 +103,10 @@ static int init(void)
     }
 
     ret = finit_module(fd, "", 0);
-    close(fd);
-    if (ret) {
+    if (ret)
         _E("Unable to load kernel module: %m");
-        return ret;
-    }
-
-    return 0;
+    close(fd);
+    return ret;
 }
 
 static void cleanup(void)
diff --git a/src/utils.c b/src/utils.c
index abb0114..ca687b8 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -41,12 +41,12 @@ char *get_basename(char *path)
     return base ? base+1 : path;
 }
 
-int pid_to_basename(int pid, char *basename)
+int pid_to_basename(int pid, char *basename, size_t basename_buflen)
 {
-    char path[PATH_MAX];
+    char path[PATH_MAX + 1];
     char link_name[NAME_MAX];
     char *basename_ptr;
-    int basename_len;
+    size_t basename_len;
     int ret;
 
     /* Assembly /proc/pid/exe */
@@ -55,8 +55,9 @@ int pid_to_basename(int pid, char *basename)
         return -1;
     }
 
-    /* Readlink /proc/pid/exe */
-    ret = readlink(link_name, path, PATH_MAX);
+    /* Readlink /proc/pid/exe (leaving space for the null terminator
+     * which readlink never appends on its own) */
+    ret = readlink(link_name, path, sizeof(path) - 1);
     if (ret <= 0) {
         _DD("readlink() failed, pid: %d, ret: %d\n", pid, ret);
         return -EEXIST;
@@ -68,7 +69,10 @@ int pid_to_basename(int pid, char *basename)
     basename_ptr = get_basename(path);
     basename_len = strlen(basename_ptr);
 
-    strncpy(basename, basename_ptr, basename_len);
+    if (basename_len >= basename_buflen)
+        return -ENAMETOOLONG;
+
+    strncpy(basename, basename_ptr, basename_buflen);
     basename[basename_len] = 0;
 
     return 0;
diff --git a/src/utils.h b/src/utils.h
index 59d2470..b9c3aca 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -39,7 +39,7 @@
 
 unsigned long long time_now(void);
 char *get_basename(char *path);
-int pid_to_basename(int pid, char *basename);
+int pid_to_basename(int pid, char *basename, size_t basename_buflen);
 const char *smpl2str(struct sample_s *s);
 
 int _json_object_object_merge(json_object *obj1, char *key, json_object *obj2_node);
-- 
2.34.1