Browse Source

[PAL] Correct memory allocation in file_rename() and dir_rename()

Use strdup() to correctly copy a string with NULL termination.
Prevent memory leak by freeing {file,dir}.realpath when needed.
Gary 5 years ago
parent
commit
4ae620f87b
3 changed files with 100 additions and 25 deletions
  1. 34 9
      Pal/src/host/FreeBSD/db_files.c
  2. 32 8
      Pal/src/host/Linux-SGX/db_files.c
  3. 34 8
      Pal/src/host/Linux/db_files.c

+ 34 - 9
Pal/src/host/FreeBSD/db_files.c

@@ -130,10 +130,11 @@ static int file_close (PAL_HANDLE handle)
 
     int ret = INLINE_SYSCALL(close, 1, fd);
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->file.realpath &&
-        handle->file.realpath != (void *) handle + HANDLE_SIZE(file))
+        handle->file.realpath != (void *) handle + HANDLE_SIZE(file)) {
         free((void *) handle->file.realpath);
-
+    }
     return IS_ERR(ret) ? unix_to_pal_error(ERRNO(ret)) : 0;
 }
 
@@ -270,12 +271,23 @@ static int file_attrsetbyhdl (PAL_HANDLE handle,
 static int file_rename (PAL_HANDLE handle, const char * type,
                         const char * uri)
 {
-    int ret = INLINE_SYSCALL(rename, 2, handle->file.realpath, uri);
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
 
-    if (IS_ERR(ret))
+    int ret = INLINE_SYSCALL(rename, 2, handle->file.realpath, uri);
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->file.realpath &&
+            handle->file.realpath != (void *) handle + HANDLE_SIZE(file)) {
+        free((void *) handle->file.realpath);
+    }
 
-    handle->file.realpath = malloc_copy(uri, strlen(uri));
+    handle->file.realpath = tmp;
     return 0;
 }
 
@@ -459,9 +471,11 @@ static int dir_close (PAL_HANDLE handle)
         handle->dir.buf = handle->dir.ptr = handle->dir.end = NULL;
     }
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->dir.realpath &&
-        handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir))
+        handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir)) {
         free((void *) handle->dir.realpath);
+    }
 
     if (IS_ERR(ret))
         return -PAL_ERROR_BADHANDLE;
@@ -489,12 +503,23 @@ static int dir_delete (PAL_HANDLE handle, int access)
 static int dir_rename (PAL_HANDLE handle, const char * type,
                        const char * uri)
 {
-    int ret = INLINE_SYSCALL(rename, 2, handle->dir.realpath, uri);
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
 
-    if (IS_ERR(ret))
+    int ret = INLINE_SYSCALL(rename, 2, handle->dir.realpath, uri);
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->dir.realpath &&
+            handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir)) {
+        free((void *) handle->dir.realpath);
+    }
 
-    handle->dir.realpath = malloc_copy(uri, strlen(uri));
+    handle->dir.realpath = tmp;
     return 0;
 }
 

+ 32 - 8
Pal/src/host/Linux-SGX/db_files.c

@@ -165,6 +165,7 @@ static int file_close (PAL_HANDLE handle)
     int fd = handle->file.fd;
     ocall_close(fd);
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->file.realpath &&
         handle->file.realpath != (void *) handle + HANDLE_SIZE(file))
         free((void *) handle->file.realpath);
@@ -356,13 +357,24 @@ static int file_rename (PAL_HANDLE handle, const char * type,
 {
     if (!strcmp_static(type, "file"))
         return -PAL_ERROR_INVAL;
+
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
+
     int ret = ocall_rename(handle->file.realpath, uri);
-    if (IS_ERR(ret))
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->file.realpath &&
+        handle->file.realpath != (void *) handle + HANDLE_SIZE(file)) {
+        free((void *) handle->file.realpath);
+    }
 
-    /* TODO: old realpath memory is potentially leaked here, and need
-     * to check for strdup memory allocation failure. */
-    handle->file.realpath = strdup(uri);
+    handle->file.realpath = tmp;
     return 0;
 }
 
@@ -533,6 +545,7 @@ static int dir_close (PAL_HANDLE handle)
         handle->dir.buf = handle->dir.ptr = handle->dir.end = (PAL_PTR) NULL;
     }
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->dir.realpath &&
         handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir))
         free((void *) handle->dir.realpath);
@@ -559,13 +572,24 @@ static int dir_rename (PAL_HANDLE handle, const char * type,
 {
     if (!strcmp_static(type, "dir"))
         return -PAL_ERROR_INVAL;
+
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
+
     int ret = ocall_rename(handle->dir.realpath, uri);
-    if (IS_ERR(ret))
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->dir.realpath &&
+        handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir)) {
+        free((void *) handle->dir.realpath);
+    }
 
-    /* TODO: old realpath memory is potentially leaked here, and need
-     * to check for strdup memory allocation failure. */
-    handle->dir.realpath = strdup(uri);
+    handle->dir.realpath = tmp;
     return 0;
 }
 

+ 34 - 8
Pal/src/host/Linux/db_files.c

@@ -133,9 +133,11 @@ static int file_close (PAL_HANDLE handle)
 
     int ret = INLINE_SYSCALL(close, 1, fd);
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->file.realpath &&
-        handle->file.realpath != (void *) handle + HANDLE_SIZE(file))
+        handle->file.realpath != (void *) handle + HANDLE_SIZE(file)) {
         free((void *) handle->file.realpath);
+    }
 
     return IS_ERR(ret) ? unix_to_pal_error(ERRNO(ret)) : 0;
 }
@@ -288,12 +290,23 @@ static int file_rename (PAL_HANDLE handle, const char * type,
     if (!strcmp_static(type, "file"))
         return -PAL_ERROR_INVAL;
 
-    int ret = INLINE_SYSCALL(rename, 2, handle->file.realpath, uri);
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
 
-    if (IS_ERR(ret))
+    int ret = INLINE_SYSCALL(rename, 2, handle->file.realpath, uri);
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->file.realpath &&
+            handle->file.realpath != (void *) handle + HANDLE_SIZE(file)) {
+        free((void *) handle->file.realpath);
+    }
 
-    handle->file.realpath = malloc_copy(uri, strlen(uri));
+    handle->file.realpath = tmp;
     return 0;
 }
 
@@ -484,9 +497,11 @@ static int dir_close (PAL_HANDLE handle)
         handle->dir.buf = handle->dir.ptr = handle->dir.end = (PAL_PTR) NULL;
     }
 
+    /* initial realpath is part of handle object and will be freed with it */
     if (handle->dir.realpath &&
-        handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir))
+        handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir)) {
         free((void *) handle->dir.realpath);
+    }
 
     if (IS_ERR(ret))
         return -PAL_ERROR_BADHANDLE;
@@ -517,12 +532,23 @@ static int dir_rename (PAL_HANDLE handle, const char * type,
     if (!strcmp_static(type, "dir"))
         return -PAL_ERROR_INVAL;
 
-    int ret = INLINE_SYSCALL(rename, 2, handle->dir.realpath, uri);
+    char* tmp = strdup(uri);
+    if (!tmp)
+        return -PAL_ERROR_NOMEM;
 
-    if (IS_ERR(ret))
+    int ret = INLINE_SYSCALL(rename, 2, handle->dir.realpath, uri);
+    if (IS_ERR(ret)) {
+        free(tmp);
         return unix_to_pal_error(ERRNO(ret));
+    }
+
+    /* initial realpath is part of handle object and will be freed with it */
+    if (handle->dir.realpath &&
+            handle->dir.realpath != (void *) handle + HANDLE_SIZE(dir)) {
+        free((void *) handle->dir.realpath);
+    }
 
-    handle->dir.realpath = malloc_copy(uri, strlen(uri));
+    handle->dir.realpath = tmp;
     return 0;
 }