Browse Source

Not clean up allocated and mapped memory resources in error handling code

The config variable points to a new allocated memory block inside of load_manifest() but it isn't cleaned up on two error handling branches.
also the mapped config_raw is not unmapped on last error handling branch before return.
There is also enclave variable points to a new allocated memory block inside of main() that does not get freed on multiple error handling branches
Gary 7 years ago
parent
commit
ae5b7a0294
1 changed files with 37 additions and 10 deletions
  1. 37 10
      Pal/src/host/Linux-SGX/sgx_main.c

+ 37 - 10
Pal/src/host/Linux-SGX/sgx_main.c

@@ -617,6 +617,7 @@ static void create_instance (struct pal_sec * pal_sec)
 
 int load_manifest (int fd, struct config_store ** config_ptr)
 {
+    int retval = -EINVAL;
     int nbytes = INLINE_SYSCALL(lseek, 3, fd, 0, SEEK_END);
 
     if (IS_ERR(nbytes))
@@ -631,8 +632,10 @@ int load_manifest (int fd, struct config_store ** config_ptr)
                            PROT_READ|PROT_WRITE, MAP_PRIVATE,
                            fd, 0);
 
-    if (IS_ERR_P(config_raw))
-        return -ERRNO_P(config_raw);
+    if (IS_ERR_P(config_raw)) {
+        retval = -ERRNO_P(config_raw);
+        goto finalize;
+    }
 
     config->raw_data = config_raw;
     config->raw_size = nbytes;
@@ -644,11 +647,21 @@ int load_manifest (int fd, struct config_store ** config_ptr)
 
     if (ret < 0) {
         SGX_DBG(DBG_E, "can't read manifest: %s\n", errstring);
-        return ret;
+        retval = ret;
+        goto finalize;
     }
 
     *config_ptr = config;
     return 0;
+
+finalize:
+    if (config) {
+        free(config);
+    }
+    if (!IS_ERR_P(config_raw)) {
+        INLINE_SYSCALL(munmap, 2, config_raw, nbytes);
+    }
+    return retval;
 }
 
 static int load_enclave (struct pal_enclave * enclave,
@@ -832,6 +845,7 @@ int main (int argc, const char ** argv, const char ** envp)
 {
     const char * manifest_uri = NULL, * exec_uri = NULL;
     const char * pal_loader = argv[0];
+    int retval = -EINVAL;
     argc--;
     argv++;
 
@@ -842,8 +856,10 @@ int main (int argc, const char ** argv, const char ** envp)
     memset(enclave, 0, sizeof(struct pal_enclave));
 
     int is_child = sgx_init_child_process(&enclave->pal_sec);
-    if (is_child < 0)
-        return is_child;
+    if (is_child < 0) {
+        retval = is_child;
+        goto finalize;
+    }
 
     if (!is_child) {
         /* occupy PROC_INIT_FD so no one will use it */
@@ -862,8 +878,10 @@ int main (int argc, const char ** argv, const char ** envp)
     }
 
     int fd = INLINE_SYSCALL(open, 3, exec_uri + 5, O_RDONLY|O_CLOEXEC, 0);
-    if (IS_ERR(fd))
-        return -ERRNO(fd);
+    if (IS_ERR(fd)) {
+        retval = -ERRNO(fd);
+        goto finalize;
+    }
 
     char filebuf[4];
     /* check if the first argument is a executable. If it is, try finding
@@ -874,8 +892,10 @@ int main (int argc, const char ** argv, const char ** envp)
     char sgx_manifest[URI_MAX];
     int len = get_base_name(exec_uri + static_strlen("file:"), sgx_manifest,
                             URI_MAX);
-    if (len < 0)
-        return len;
+    if (len < 0) {
+        retval = len;
+        goto finalize;
+    }
 
     if (strcmp_static(sgx_manifest + len - strlen(".manifest"), ".manifest")) {
         strcpy_static(sgx_manifest + len, ".sgx", URI_MAX - len);
@@ -905,7 +925,14 @@ int main (int argc, const char ** argv, const char ** envp)
 
 usage:
     SGX_DBG(DBG_E, "USAGE: %s [executable|manifest] args ...\n", pal_loader);
-    return -EINVAL;
+    retval = -EINVAL;
+    goto finalize;
+
+finalize:
+    if (enclave) {
+        free(enclave);
+    }
+    return retval;
 }
 
 int pal_init_enclave (const char * manifest_uri,