Cleanup and sanity-checking of OOB accesses and pointer-to-local issues (Coverity...

[palacios.git] / linux_module / iface-file.c

diff --git a/linux_module/iface-file.c b/linux_module/iface-file.c
index da5aabb..67a4e73 100644 (file)
--- a/linux_module/iface-file.c
+++ b/linux_module/iface-file.c
@@ -56,6 +56,7 @@ static int mkdir_recursive(const char * path, unsigned short perms) {
     }
 
     memset(tmp_str, 0, strlen(path) + 1);
+    // will terminate tmp_str
     strncpy(tmp_str, path, strlen(path));
 
     dirname_ptr = tmp_str;
@@ -118,6 +119,9 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int
     /* It only exists to provide version compatibility */
     struct path tmp_path; 
 #endif
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,41)
+    struct nameidata nd;
+#endif
 
     struct path * path_ptr = NULL;
     struct dentry * dentry;
@@ -132,8 +136,6 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int
     /* Before Linux 3.1 this was somewhat more difficult */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,41)
     {
-       struct nameidata nd;
-
        // I'm not 100% sure about the version here, but it was around this time that the API changed
 #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,38) 
        ret = kern_path_parent(pathname, &nd);
@@ -161,7 +163,7 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int
     {
        dentry = kern_path_create(AT_FDCWD, pathname, &tmp_path, 1);
        
-       if (IS_ERR(dentry)) {
+       if (!dentry || IS_ERR(dentry)) {
            return 0;
        }
        
@@ -170,7 +172,7 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int
 #endif    
 
 
-    if (!IS_ERR(dentry)) {
+    if (!(!dentry || IS_ERR(dentry))) {
        ret = vfs_mkdir(path_ptr->dentry->d_inode, dentry, perms);
     }
 
@@ -218,15 +220,15 @@ static void * palacios_file_open(const char * path, int mode, void * private_dat
     pfile->mode |= O_LARGEFILE;
 
 
-    pfile->filp = filp_open(path, pfile->mode, 0);
+    pfile->filp = filp_open(path, pfile->mode, 0600);  // rw------- to start
     
-    if (IS_ERR(pfile->filp)) {
+    if (!pfile->filp || IS_ERR(pfile->filp)) {
        ERROR("Cannot open file: %s\n", path);
        palacios_free(pfile);
        return NULL;
     }
 
-    pfile->path = palacios_alloc(strlen(path));
+    pfile->path = palacios_alloc(strlen(path) + 1);
     
     if (!pfile->path) { 
        ERROR("Cannot allocate in file open\n");
@@ -234,7 +236,7 @@ static void * palacios_file_open(const char * path, int mode, void * private_dat
        palacios_free(pfile);
        return NULL;
     }
-    strncpy(pfile->path, path, strlen(path));
+    strncpy(pfile->path, path, strlen(path)); // will terminate pfile->path
     pfile->guest = guest;
     
     palacios_spinlock_init(&(pfile->lock));
@@ -252,10 +254,16 @@ static void * palacios_file_open(const char * path, int mode, void * private_dat
 static int palacios_file_close(void * file_ptr) {
     struct palacios_file * pfile = (struct palacios_file *)file_ptr;
 
+    if (!pfile) {
+        return -1;
+    }
+
     filp_close(pfile->filp, NULL);
     
     list_del(&(pfile->file_node));
 
+    palacios_spinlock_deinit(&(pfile->lock));
+
     palacios_free(pfile->path);    
     palacios_free(pfile);