You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by xy...@apache.org on 2018/05/09 17:40:54 UTC

[16/26] hadoop git commit: YARN-8207. Docker container launch use popen have risk of shell expansion. Contributed by Eric Yang.

YARN-8207. Docker container launch use popen have risk of shell expansion. Contributed by Eric Yang.


Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/a2ea7564
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/a2ea7564
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/a2ea7564

Branch: refs/heads/HDDS-4
Commit: a2ea7564209dce896a5badee3949ee41e4bc00bf
Parents: 6b96a73
Author: Jason Lowe <jl...@apache.org>
Authored: Tue May 8 15:30:36 2018 -0500
Committer: Jason Lowe <jl...@apache.org>
Committed: Tue May 8 15:30:36 2018 -0500

----------------------------------------------------------------------
 .../impl/container-executor.c                   | 118 ++--
 .../impl/container-executor.h                   |   5 +
 .../container-executor/impl/utils/docker-util.c | 611 +++++++++----------
 .../container-executor/impl/utils/docker-util.h |  38 +-
 .../impl/utils/string-utils.c                   |  24 +
 .../impl/utils/string-utils.h                   |   4 +
 .../test/utils/test_docker_util.cc              | 463 +++++++-------
 7 files changed, 674 insertions(+), 589 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
index 6b4ec0c..c5adbe4 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c
@@ -20,6 +20,7 @@
 #include "container-executor.h"
 #include "utils/docker-util.h"
 #include "utils/path-utils.h"
+#include "utils/string-utils.h"
 #include "util.h"
 #include "config.h"
 
@@ -737,7 +738,6 @@ static int create_container_directories(const char* user, const char *app_id,
     result = OUT_OF_MEMORY;
   } else {
     sprintf(combined_name, "%s/%s", app_id, container_id);
-
     char* const* log_dir_ptr;
     for(log_dir_ptr = log_dir; *log_dir_ptr != NULL; ++log_dir_ptr) {
       char *container_log_dir = get_app_log_directory(*log_dir_ptr, combined_name);
@@ -753,10 +753,10 @@ static int create_container_directories(const char* user, const char *app_id,
         free(combined_name);
         return OUT_OF_MEMORY;
       } else if (mkdirs(container_log_dir, perms) != 0) {
-    	free(container_log_dir);
+        free(container_log_dir);
       } else {
-    	result = 0;
-    	free(container_log_dir);
+        free(container_log_dir);
+        result = 0;
       }
     }
     free(combined_name);
@@ -799,7 +799,7 @@ static struct passwd* get_user_info(const char* user) {
         string_size, &result) != 0) {
     free(buffer);
     fprintf(LOGFILE, "Can't get user information %s - %s\n", user,
-	    strerror(errno));
+           strerror(errno));
     return NULL;
   }
   return result;
@@ -1094,7 +1094,6 @@ int initialize_user(const char *user, char* const* local_dirs) {
 }
 
 int create_log_dirs(const char *app_id, char * const * log_dirs) {
-
   char* const* log_root;
   char *any_one_app_log_dir = NULL;
   for(log_root=log_dirs; *log_root != NULL; ++log_root) {
@@ -1275,11 +1274,9 @@ int initialize_app(const char *user, const char *app_id,
   return -1;
 }
 
-char *construct_docker_command(const char *command_file) {
+char **construct_docker_command(const char *command_file) {
   int ret = 0;
-  size_t command_size = MIN(sysconf(_SC_ARG_MAX), 128*1024);
-  char *buffer = alloc_and_clear_memory(command_size, sizeof(char));
-
+  struct args buffer = ARGS_INITIAL_VALUE;
   uid_t user = geteuid();
   gid_t group = getegid();
   if (change_effective_user(nm_uid, nm_gid) != 0) {
@@ -1287,8 +1284,7 @@ char *construct_docker_command(const char *command_file) {
     fflush(ERRORFILE);
     exit(SETUID_OPER_FAILED);
   }
-
-  ret = get_docker_command(command_file, &CFG, buffer, command_size);
+  ret = get_docker_command(command_file, &CFG, &buffer);
   if (ret != 0) {
     fprintf(ERRORFILE, "Error constructing docker command, docker error code=%d, error message='%s'\n", ret,
             get_docker_error_message(ret));
@@ -1302,31 +1298,24 @@ char *construct_docker_command(const char *command_file) {
     exit(SETUID_OPER_FAILED);
   }
 
-  return buffer;
+  char** copy = extract_execv_args(&buffer);
+  return copy;
 }
 
 int run_docker(const char *command_file) {
-  char* docker_command = construct_docker_command(command_file);
+  char **args = construct_docker_command(command_file);
   char* docker_binary = get_docker_binary(&CFG);
-  size_t command_size = MIN(sysconf(_SC_ARG_MAX), 128*1024);
-
-  char* docker_command_with_binary = alloc_and_clear_memory(command_size, sizeof(char));
-  snprintf(docker_command_with_binary, command_size, "%s %s", docker_binary, docker_command);
-  fprintf(LOGFILE, "Invoking '%s'\n", docker_command_with_binary);
-  char **args = split_delimiter(docker_command_with_binary, " ");
-
   int exit_code = -1;
   if (execvp(docker_binary, args) != 0) {
     fprintf(ERRORFILE, "Couldn't execute the container launch with args %s - %s",
               docker_binary, strerror(errno));
-      fflush(LOGFILE);
-      fflush(ERRORFILE);
-      free(docker_binary);
-      free(args);
-      free(docker_command_with_binary);
-      free(docker_command);
-      exit_code = DOCKER_RUN_FAILED;
+    fflush(LOGFILE);
+    fflush(ERRORFILE);
+    free(docker_binary);
+    free_values(args);
+    exit_code = DOCKER_RUN_FAILED;
   } else {
+    free_values(args);
     exit_code = 0;
   }
   return exit_code;
@@ -1452,7 +1441,7 @@ int create_local_dirs(const char * user, const char *app_id,
 
   // Copy script file with permissions 700
   if (copy_file(container_file_source, script_name, script_file_dest,S_IRWXU) != 0) {
-    fprintf(ERRORFILE, "Could not create copy file %d %s\n", container_file_source, script_file_dest);
+    fprintf(ERRORFILE, "Could not create copy file %s %s (%d)\n", script_name, script_file_dest, container_file_source);
     fflush(ERRORFILE);
     exit_code = COULD_NOT_CREATE_SCRIPT_COPY;
     goto cleanup;
@@ -1513,25 +1502,15 @@ int launch_docker_container_as_user(const char * user, const char *app_id,
   char *cred_file_dest = NULL;
   char *exit_code_file = NULL;
   char *docker_command_with_binary = NULL;
-  char *docker_wait_command = NULL;
   char *docker_inspect_command = NULL;
-  char *docker_rm_command = NULL;
   char *docker_inspect_exitcode_command = NULL;
   int container_file_source =-1;
   int cred_file_source = -1;
 
-  size_t command_size = MIN(sysconf(_SC_ARG_MAX), 128*1024);
-
-  docker_command_with_binary = (char *) alloc_and_clear_memory(command_size, sizeof(char));
-  docker_wait_command = (char *) alloc_and_clear_memory(command_size, sizeof(char));
-  docker_inspect_command = (char *) alloc_and_clear_memory(command_size, sizeof(char));
-  docker_rm_command = (char *) alloc_and_clear_memory(command_size, sizeof(char));
-  docker_inspect_exitcode_command = (char *) alloc_and_clear_memory(command_size, sizeof(char));
-
   gid_t user_gid = getegid();
   uid_t prev_uid = geteuid();
 
-  char *docker_command = NULL;
+  char **docker_command = NULL;
   char *docker_binary = NULL;
 
   fprintf(LOGFILE, "Creating script paths...\n");
@@ -1581,21 +1560,31 @@ int launch_docker_container_as_user(const char * user, const char *app_id,
     goto cleanup;
   }
 
-  snprintf(docker_command_with_binary, command_size, "%s %s", docker_binary, docker_command);
+  docker_command_with_binary = flatten(docker_command);
 
-  fprintf(LOGFILE, "Launching docker container...\n");
-  fprintf(LOGFILE, "Docker run command: %s\n", docker_command_with_binary);
-  FILE* start_docker = popen(docker_command_with_binary, "r");
-  if (WEXITSTATUS(pclose (start_docker)) != 0)
-  {
+  // Launch container
+  pid_t child_pid = fork();
+  if (child_pid == -1) {
     fprintf (ERRORFILE,
-     "Could not invoke docker %s.\n", docker_command_with_binary);
+      "Could not invoke docker %s.\n", docker_command_with_binary);
     fflush(ERRORFILE);
     exit_code = UNABLE_TO_EXECUTE_CONTAINER_SCRIPT;
     goto cleanup;
   }
 
-  snprintf(docker_inspect_command, command_size,
+  if (child_pid == 0) {
+    execvp(docker_binary, docker_command);
+    fprintf(ERRORFILE, "failed to execute docker command! error: %s\n", strerror(errno));
+    return UNABLE_TO_EXECUTE_CONTAINER_SCRIPT;
+  } else {
+    exit_code = wait_and_get_exit_code(child_pid);
+    if (exit_code != 0) {
+      exit_code = UNABLE_TO_EXECUTE_CONTAINER_SCRIPT;
+      goto cleanup;
+    }
+  }
+
+  docker_inspect_command = make_string(
     "%s inspect --format {{.State.Pid}} %s",
     docker_binary, container_id);
 
@@ -1643,10 +1632,10 @@ int launch_docker_container_as_user(const char * user, const char *app_id,
     }
 
     fprintf(LOGFILE, "Waiting for docker container to finish.\n");
+
+    // wait for pid to finish
 #ifdef __linux
-    size_t command_size = MIN(sysconf(_SC_ARG_MAX), 128*1024);
-    char* proc_pid_path = alloc_and_clear_memory(command_size, sizeof(char));
-    snprintf(proc_pid_path, command_size, "%s/%d", PROC_PATH, pid);
+    char* proc_pid_path = make_string("%s/%d", PROC_PATH, pid);
     while (dir_exists(proc_pid_path) == 0) {
       sleep(1);
     }
@@ -1661,7 +1650,8 @@ int launch_docker_container_as_user(const char * user, const char *app_id,
 #endif
   }
 
-  sprintf(docker_inspect_exitcode_command,
+  // discover container exit code
+  docker_inspect_exitcode_command = make_string(
     "%s inspect --format {{.State.ExitCode}} %s",
   docker_binary, container_id);
   fprintf(LOGFILE, "Obtaining the exit code...\n");
@@ -1713,9 +1703,8 @@ cleanup:
   free(script_file_dest);
   free(cred_file_dest);
   free(docker_command_with_binary);
-  free(docker_wait_command);
   free(docker_inspect_command);
-  free(docker_rm_command);
+  free_values(docker_command);
   return exit_code;
 }
 
@@ -2404,3 +2393,24 @@ int traffic_control_read_stats(char *command_file) {
 struct configuration* get_cfg() {
   return &CFG;
 }
+
+/**
+ * Flatten docker launch command
+ */
+char* flatten(char **args) {
+  size_t total = 1;
+  for (int i = 0; args[i] != NULL; i++) {
+    total = total + strlen(args[i]) + 1;
+  }
+  char *buffer = (char *) malloc(total * sizeof(char));
+  char *to = NULL;
+  to = buffer;
+  for (int i = 0; args[i] != NULL; i++) {
+    to = stpcpy(to, args[i]);
+    to = stpcpy(to, " ");
+  }
+  *to = '\0';
+  return buffer;
+}
+
+

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
index 47c4221..9136606 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.h
@@ -284,3 +284,8 @@ int execute_regex_match(const char *regex_str, const char *input);
 int validate_docker_image_name(const char *image_name);
 
 struct configuration* get_cfg();
+
+/**
+ * Flatten docker launch command
+ */
+char* flatten(char **args);

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c
index 6795bd8..8cd59f7 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.c
@@ -47,36 +47,63 @@ static int read_and_verify_command_file(const char *command_file, const char *do
   return ret;
 }
 
-static int add_to_buffer(char *buff, const size_t bufflen, const char *string) {
-  size_t current_len = strlen(buff);
-  size_t string_len = strlen(string);
-  if (current_len + string_len < bufflen - 1) {
-    strncpy(buff + current_len, string, string_len);
-    buff[current_len + string_len] = '\0';
-    return 0;
+static int add_to_args(args *args, const char *string) {
+  if (string == NULL) {
+    return -1;
+  }
+  if (args->data == NULL || args->length >= DOCKER_ARG_MAX) {
+    return -1;
+  }
+  char *clone = strdup(string);
+  if (clone == NULL) {
+    return -1;
+  }
+  if (args->data != NULL) {
+    args->data[args->length] = clone;
+    args->length++;
+  }
+  return 0;
+}
+
+void reset_args(args *args) {
+  int i = 0;
+  if (args == NULL) {
+    return;
+  }
+  for (i = 0; i < args->length; i++) {
+    free(args->data[i]);
+  }
+  args->length = 0;
+}
+
+char** extract_execv_args(args* args) {
+  char** copy = (char**)malloc((args->length + 1) * sizeof(char*));
+  for (int i = 0; i < args->length; i++) {
+    copy[i] = args->data[i];
   }
-  return -1;
+  copy[args->length] = NULL;
+  args->length = 0;
+  return copy;
 }
 
 static int add_param_to_command(const struct configuration *command_config, const char *key, const char *param,
-                                const int with_argument, char *out, const size_t outlen) {
-  size_t tmp_buffer_size = 4096;
+                                const int with_argument, args *args) {
   int ret = 0;
-  char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
+  char *tmp_buffer = NULL;
   char *value = get_configuration_value(key, DOCKER_COMMAND_FILE_SECTION, command_config);
   if (value != NULL) {
     if (with_argument) {
-      quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, param, value);
-      ret = add_to_buffer(out, outlen, tmp_buffer);
+      tmp_buffer = make_string("%s%s", param, value);
+      ret = add_to_args(args, tmp_buffer);
+      free(tmp_buffer);
     } else if (strcmp(value, "true") == 0) {
-      ret = add_to_buffer(out, outlen, param);
+      ret = add_to_args(args, param);
     }
     free(value);
     if (ret != 0) {
       ret = BUFFER_TOO_SMALL;
     }
   }
-  free(tmp_buffer);
   return ret;
 }
 
@@ -116,8 +143,8 @@ int check_trusted_image(const struct configuration *command_config, const struct
   }
   free(image_name);
 
-  free_and_exit:
-  free(privileged_registry);
+free_and_exit:
+  free_values(privileged_registry);
   return ret;
 }
 
@@ -141,9 +168,8 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c
                                            const struct configuration *executor_cfg,
                                            const char *key, const char *allowed_key, const char *param,
                                            const int multiple_values, const char prefix,
-                                           char *out, const size_t outlen) {
-  size_t tmp_buffer_size = 4096;
-  char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
+                                           args *args) {
+  char *tmp_buffer = NULL;
   char *tmp_ptr = NULL;
   char **values = NULL;
   char **permitted_values = get_configuration_values_delimiter(allowed_key,
@@ -174,7 +200,6 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c
     if (permitted_values != NULL) {
       // Values are user requested.
       for (i = 0; values[i] != NULL; ++i) {
-        memset(tmp_buffer, 0, tmp_buffer_size);
         permitted = 0;
         if(prefix != 0) {
           tmp_ptr = strchr(values[i], prefix);
@@ -195,12 +220,8 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c
           } else {
             // If permitted-Values[j] is a REGEX, use REGEX to compare
             if (is_regex(permitted_values[j])) {
-              size_t offset = tmp_ptr - values[i];
-              dst = (char *) alloc_and_clear_memory(offset, sizeof(char));
-              strncpy(dst, values[i], offset);
-              dst[tmp_ptr - values[i]] = '\0';
-              pattern = (char *) alloc_and_clear_memory((size_t)(strlen(permitted_values[j]) - 6), sizeof(char));
-              strcpy(pattern, permitted_values[j] + 6);
+              dst = strndup(values[i], tmp_ptr - values[i]);
+              pattern = strdup(permitted_values[j] + 6);
               ret = execute_regex_match(pattern, dst);
             } else {
               ret = strncmp(values[i], permitted_values[j], tmp_ptr - values[i]);
@@ -214,8 +235,9 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c
           }
         }
         if (permitted == 1) {
-          quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, param, values[i]);
-          ret = add_to_buffer(out, outlen, tmp_buffer);
+          tmp_buffer = make_string("%s%s", param, values[i]);
+          ret = add_to_args(args, tmp_buffer);
+          free(tmp_buffer);
           if (ret != 0) {
             fprintf(ERRORFILE, "Output buffer too small\n");
             ret = BUFFER_TOO_SMALL;
@@ -238,15 +260,11 @@ static int add_param_to_command_if_allowed(const struct configuration *command_c
   free_and_exit:
   free_values(values);
   free_values(permitted_values);
-  free(tmp_buffer);
-  if (ret != 0) {
-    memset(out, 0, outlen);
-  }
   return ret;
 }
 
-static int add_docker_config_param(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "docker-config", "--config=", 1, out, outlen);
+static int add_docker_config_param(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "docker-config", "--config=", 1, args);
 }
 
 static int validate_volume_name(const char *volume_name) {
@@ -338,7 +356,7 @@ int docker_module_enabled(const struct configuration *conf) {
   return 0;
 }
 
-int get_docker_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
   struct configuration command_config = {0, NULL};
 
@@ -349,23 +367,23 @@ int get_docker_command(const char *command_file, const struct configuration *con
 
   char *command = get_configuration_value("docker-command", DOCKER_COMMAND_FILE_SECTION, &command_config);
   if (strcmp(DOCKER_INSPECT_COMMAND, command) == 0) {
-    return get_docker_inspect_command(command_file, conf, out, outlen);
+    return get_docker_inspect_command(command_file, conf, args);
   } else if (strcmp(DOCKER_KILL_COMMAND, command) == 0) {
-    return get_docker_kill_command(command_file, conf, out, outlen);
+    return get_docker_kill_command(command_file, conf, args);
   } else if (strcmp(DOCKER_LOAD_COMMAND, command) == 0) {
-    return get_docker_load_command(command_file, conf, out, outlen);
+    return get_docker_load_command(command_file, conf, args);
   } else if (strcmp(DOCKER_PULL_COMMAND, command) == 0) {
-    return get_docker_pull_command(command_file, conf, out, outlen);
+    return get_docker_pull_command(command_file, conf, args);
   } else if (strcmp(DOCKER_RM_COMMAND, command) == 0) {
-    return get_docker_rm_command(command_file, conf, out, outlen);
+    return get_docker_rm_command(command_file, conf, args);
   } else if (strcmp(DOCKER_RUN_COMMAND, command) == 0) {
-    return get_docker_run_command(command_file, conf, out, outlen);
+    return get_docker_run_command(command_file, conf, args);
   } else if (strcmp(DOCKER_STOP_COMMAND, command) == 0) {
-    return get_docker_stop_command(command_file, conf, out, outlen);
+    return get_docker_stop_command(command_file, conf, args);
   } else if (strcmp(DOCKER_VOLUME_COMMAND, command) == 0) {
-    return get_docker_volume_command(command_file, conf, out, outlen);
+    return get_docker_volume_command(command_file, conf, args);
   } else if (strcmp(DOCKER_START_COMMAND, command) == 0) {
-    return get_docker_start_command(command_file, conf, out, outlen);
+    return get_docker_start_command(command_file, conf, args);
   } else {
     return UNKNOWN_DOCKER_COMMAND;
   }
@@ -397,10 +415,9 @@ static int value_permitted(const struct configuration* executor_cfg,
   return found;
 }
 
-int get_docker_volume_command(const char *command_file, const struct configuration *conf, char *out,
-                               const size_t outlen) {
+int get_docker_volume_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
-  char *driver = NULL, *volume_name = NULL, *sub_command = NULL, *format = NULL;
+  char *driver = NULL, *volume_name = NULL, *sub_command = NULL, *format = NULL, *docker = NULL;
   struct configuration command_config = {0, NULL};
   ret = read_and_verify_command_file(command_file, DOCKER_VOLUME_COMMAND, &command_config);
   if (ret != 0) {
@@ -416,15 +433,20 @@ int get_docker_volume_command(const char *command_file, const struct configurati
     goto cleanup;
   }
 
-  memset(out, 0, outlen);
+  docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  if (ret != 0) {
+    ret = BUFFER_TOO_SMALL;
+    goto cleanup;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     ret = BUFFER_TOO_SMALL;
     goto cleanup;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_VOLUME_COMMAND);
+  ret = add_to_args(args, DOCKER_VOLUME_COMMAND);
   if (ret != 0) {
     goto cleanup;
   }
@@ -443,18 +465,16 @@ int get_docker_volume_command(const char *command_file, const struct configurati
       goto cleanup;
     }
 
-    ret = add_to_buffer(out, outlen, " create");
+    ret = add_to_args(args, "create");
     if (ret != 0) {
       goto cleanup;
     }
 
-    ret = add_to_buffer(out, outlen, " --name=");
-    if (ret != 0) {
-      goto cleanup;
-    }
-
-    ret = add_to_buffer(out, outlen, volume_name);
+    char *name_buffer = make_string("--name=%s", volume_name);
+    ret = add_to_args(args, name_buffer);
+    free(name_buffer);
     if (ret != 0) {
+      ret = BUFFER_TOO_SMALL;
       goto cleanup;
     }
 
@@ -465,30 +485,26 @@ int get_docker_volume_command(const char *command_file, const struct configurati
       goto cleanup;
     }
 
-    ret = add_to_buffer(out, outlen, " --driver=");
-    if (ret != 0) {
-      goto cleanup;
-    }
-
-    ret = add_to_buffer(out, outlen, driver);
+    char *driver_buffer = make_string("--driver=%s", driver);
+    ret = add_to_args(args, driver_buffer);
+    free(driver_buffer);
     if (ret != 0) {
+      ret = BUFFER_TOO_SMALL;
       goto cleanup;
     }
   } else if (0 == strcmp(sub_command, "ls")) {
     format = get_configuration_value("format", DOCKER_COMMAND_FILE_SECTION, &command_config);
 
-    ret = add_to_buffer(out, outlen, " ls");
+    ret = add_to_args(args, "ls");
     if (ret != 0) {
       goto cleanup;
     }
-
     if (format) {
-      ret = add_to_buffer(out, outlen, " --format=");
-      if (ret != 0) {
-        goto cleanup;
-      }
-      ret = add_to_buffer(out, outlen, format);
+      char *tmp_buffer = make_string("--format=%s", format);
+      ret = add_to_args(args, tmp_buffer);
+      free(tmp_buffer);
       if (ret != 0) {
+        ret = BUFFER_TOO_SMALL;
         goto cleanup;
       }
     }
@@ -499,20 +515,15 @@ cleanup:
   free(volume_name);
   free(sub_command);
   free(format);
-
-  // clean up out buffer
-  if (ret != 0) {
-    out[0] = 0;
-  }
+  free(docker);
   return ret;
 }
 
-int get_docker_inspect_command(const char *command_file, const struct configuration *conf, char *out,
-                               const size_t outlen) {
+int get_docker_inspect_command(const char *command_file, const struct configuration *conf, args *args) {
   const char *valid_format_strings[] = { "{{.State.Status}}",
                                 "{{range(.NetworkSettings.Networks)}}{{.IPAddress}},{{end}}{{.Config.Hostname}}" };
   int ret = 0, i = 0, valid_format = 0;
-  char *format = NULL, *container_name = NULL;
+  char *format = NULL, *container_name = NULL, *tmp_buffer = NULL;
   struct configuration command_config = {0, NULL};
   ret = read_and_verify_command_file(command_file, DOCKER_INSPECT_COMMAND, &command_config);
   if (ret != 0) {
@@ -542,32 +553,30 @@ int get_docker_inspect_command(const char *command_file, const struct configurat
     return INVALID_DOCKER_INSPECT_FORMAT;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  if (ret != 0) {
+    goto free_and_exit;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     free(container_name);
     free(format);
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_INSPECT_COMMAND);
+  ret = add_to_args(args, DOCKER_INSPECT_COMMAND);
   if (ret != 0) {
     goto free_and_exit;
   }
-  ret = add_to_buffer(out, outlen, " --format=");
-  if (ret != 0) {
-    goto free_and_exit;
-  }
-  ret = add_to_buffer(out, outlen, format);
-  if (ret != 0) {
-    goto free_and_exit;
-  }
-  ret = add_to_buffer(out, outlen, " ");
+  tmp_buffer = make_string("--format=%s", format);
+  ret = add_to_args(args, tmp_buffer);
+  free(tmp_buffer);
   if (ret != 0) {
     goto free_and_exit;
   }
-  ret = add_to_buffer(out, outlen, container_name);
+  ret = add_to_args(args, container_name);
   if (ret != 0) {
     goto free_and_exit;
   }
@@ -578,14 +587,13 @@ int get_docker_inspect_command(const char *command_file, const struct configurat
   free_and_exit:
   free(format);
   free(container_name);
+  free(docker);
   return BUFFER_TOO_SMALL;
 }
 
-int get_docker_load_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_load_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
   char *image_name = NULL;
-  size_t tmp_buffer_size = 1024;
-  char *tmp_buffer = NULL;
   struct configuration command_config = {0, NULL};
   ret = read_and_verify_command_file(command_file, DOCKER_LOAD_COMMAND, &command_config);
   if (ret != 0) {
@@ -597,19 +605,23 @@ int get_docker_load_command(const char *command_file, const struct configuration
     return INVALID_DOCKER_IMAGE_NAME;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  free(docker);
+  if (ret != 0) {
+    return BUFFER_TOO_SMALL;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     free(image_name);
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_LOAD_COMMAND);
+  ret = add_to_args(args, DOCKER_LOAD_COMMAND);
   if (ret == 0) {
-    tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
-    quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, " --i=", image_name);
-    ret = add_to_buffer(out, outlen, tmp_buffer);
+    char *tmp_buffer = make_string("--i=%s", image_name);
+    ret = add_to_args(args, tmp_buffer);
     free(tmp_buffer);
     free(image_name);
     if (ret != 0) {
@@ -626,11 +638,9 @@ static int validate_docker_image_name(const char *image_name) {
   return execute_regex_match(regex_str, image_name);
 }
 
-int get_docker_pull_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_pull_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
   char *image_name = NULL;
-  size_t tmp_buffer_size = 1024;
-  char *tmp_buffer = NULL;
   struct configuration command_config = {0, NULL};
   ret = read_and_verify_command_file(command_file, DOCKER_PULL_COMMAND, &command_config);
   if (ret != 0) {
@@ -642,30 +652,33 @@ int get_docker_pull_command(const char *command_file, const struct configuration
     return INVALID_DOCKER_IMAGE_NAME;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  if (ret != 0) {
+    goto free_pull;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
-    return BUFFER_TOO_SMALL;
+    goto free_pull;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_PULL_COMMAND);
+  ret = add_to_args(args, DOCKER_PULL_COMMAND);
   if (ret == 0) {
-    tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
-    quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, " ", image_name);
-    ret = add_to_buffer(out, outlen, tmp_buffer);
-    free(tmp_buffer);
+    ret = add_to_args(args, image_name);
     free(image_name);
     if (ret != 0) {
-      return BUFFER_TOO_SMALL;
+      goto free_pull;
     }
     return 0;
   }
+  free_pull:
   free(image_name);
+  free(docker);
   return BUFFER_TOO_SMALL;
 }
 
-int get_docker_rm_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_rm_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
   char *container_name = NULL;
   struct configuration command_config = {0, NULL};
@@ -679,19 +692,21 @@ int get_docker_rm_command(const char *command_file, const struct configuration *
     return INVALID_DOCKER_CONTAINER_NAME;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  free(docker);
+  if (ret != 0) {
+    return BUFFER_TOO_SMALL;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_RM_COMMAND);
+  ret = add_to_args(args, DOCKER_RM_COMMAND);
   if (ret == 0) {
-    ret = add_to_buffer(out, outlen, " ");
-    if (ret == 0) {
-      ret = add_to_buffer(out, outlen, container_name);
-    }
+    ret = add_to_args(args, container_name);
     free(container_name);
     if (ret != 0) {
       return BUFFER_TOO_SMALL;
@@ -703,7 +718,7 @@ int get_docker_rm_command(const char *command_file, const struct configuration *
 }
 
 int get_docker_stop_command(const char *command_file, const struct configuration *conf,
-                            char *out, const size_t outlen) {
+                            args *args) {
   int ret = 0;
   size_t len = 0, i = 0;
   char *value = NULL;
@@ -719,14 +734,19 @@ int get_docker_stop_command(const char *command_file, const struct configuration
     return INVALID_DOCKER_CONTAINER_NAME;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  free(docker);
+  if (ret != 0) {
+    goto free_and_exit;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
-    return BUFFER_TOO_SMALL;
+    goto free_and_exit;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_STOP_COMMAND);
+  ret = add_to_args(args, DOCKER_STOP_COMMAND);
   if (ret == 0) {
     value = get_configuration_value("time", DOCKER_COMMAND_FILE_SECTION, &command_config);
     if (value != NULL) {
@@ -734,36 +754,26 @@ int get_docker_stop_command(const char *command_file, const struct configuration
       for (i = 0; i < len; ++i) {
         if (isdigit(value[i]) == 0) {
           fprintf(ERRORFILE, "Value for time is not a number '%s'\n", value);
-          free(container_name);
-          memset(out, 0, outlen);
-          return INVALID_DOCKER_STOP_COMMAND;
+          ret = INVALID_DOCKER_STOP_COMMAND;
+          goto free_and_exit;
         }
       }
-      ret = add_to_buffer(out, outlen, " --time=");
-      if (ret == 0) {
-        ret = add_to_buffer(out, outlen, value);
-      }
+      char *time_buffer = make_string("--time=%s", value);
+      ret = add_to_args(args, time_buffer);
+      free(time_buffer);
       if (ret != 0) {
-        free(container_name);
-        return BUFFER_TOO_SMALL;
+        goto free_and_exit;
       }
     }
-    ret = add_to_buffer(out, outlen, " ");
-    if (ret == 0) {
-      ret = add_to_buffer(out, outlen, container_name);
-    }
-    free(container_name);
-    if (ret != 0) {
-      return BUFFER_TOO_SMALL;
-    }
-    return 0;
+    ret = add_to_args(args, container_name);
   }
+free_and_exit:
   free(container_name);
-  return BUFFER_TOO_SMALL;
+  return ret;
 }
 
 int get_docker_kill_command(const char *command_file, const struct configuration *conf,
-                            char *out, const size_t outlen) {
+                            args *args) {
   int ret = 0;
   size_t len = 0, i = 0;
   char *value = NULL;
@@ -779,14 +789,19 @@ int get_docker_kill_command(const char *command_file, const struct configuration
     return INVALID_DOCKER_CONTAINER_NAME;
   }
 
-  memset(out, 0, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  free(docker);
+  if (ret != 0) {
+    return BUFFER_TOO_SMALL;
+  }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_KILL_COMMAND);
+  ret = add_to_args(args, DOCKER_KILL_COMMAND);
   if (ret == 0) {
     value = get_configuration_value("signal", DOCKER_COMMAND_FILE_SECTION, &command_config);
     if (value != NULL) {
@@ -794,35 +809,26 @@ int get_docker_kill_command(const char *command_file, const struct configuration
       for (i = 0; i < len; ++i) {
         if (isupper(value[i]) == 0) {
           fprintf(ERRORFILE, "Value for signal contains non-uppercase characters '%s'\n", value);
-          free(container_name);
-          memset(out, 0, outlen);
-          return INVALID_DOCKER_KILL_COMMAND;
+          ret = INVALID_DOCKER_KILL_COMMAND;
+          goto free_and_exit;
         }
       }
-      ret = add_to_buffer(out, outlen, " --signal=");
-      if (ret == 0) {
-        ret = add_to_buffer(out, outlen, value);
-      }
+
+      char *signal_buffer = make_string("--signal=%s", value);
+      ret = add_to_args(args, signal_buffer);
+      free(signal_buffer);
       if (ret != 0) {
-        free(container_name);
-        return BUFFER_TOO_SMALL;
+        goto free_and_exit;
       }
     }
-    ret = add_to_buffer(out, outlen, " ");
-    if (ret == 0) {
-      ret = add_to_buffer(out, outlen, container_name);
-    }
-    free(container_name);
-    if (ret != 0) {
-      return BUFFER_TOO_SMALL;
-    }
-    return 0;
+    ret = add_to_args(args, container_name);
   }
+free_and_exit:
   free(container_name);
-  return BUFFER_TOO_SMALL;
+  return ret;
 }
 
-int get_docker_start_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_start_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0;
   char *container_name = NULL;
   struct configuration command_config = {0, NULL};
@@ -836,99 +842,86 @@ int get_docker_start_command(const char *command_file, const struct configuratio
     return INVALID_DOCKER_CONTAINER_NAME;
   }
 
-  memset(out, 0, outlen);
-
-  ret = add_docker_config_param(&command_config, out, outlen);
+  ret = add_docker_config_param(&command_config, args);
   if (ret != 0) {
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_START_COMMAND);
-  if (ret != 0) {
-    goto free_and_exit;
-  }
-  ret = add_to_buffer(out, outlen, " ");
-  if (ret != 0) {
-    goto free_and_exit;
-  }
-  ret = add_to_buffer(out, outlen, container_name);
+  ret = add_to_args(args, DOCKER_START_COMMAND);
   if (ret != 0) {
     goto free_and_exit;
   }
+  ret = add_to_args(args, container_name);
 free_and_exit:
   free(container_name);
   return ret;
 }
 
-static int detach_container(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "detach", "-d ", 0, out, outlen);
+static int detach_container(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "detach", "-d", 0, args);
 }
 
-static int  rm_container_on_exit(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "rm", "--rm ", 0, out, outlen);
+static int  rm_container_on_exit(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "rm", "--rm", 0, args);
 }
 
-static int set_container_workdir(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "workdir", "--workdir=", 1, out, outlen);
+static int set_container_workdir(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "workdir", "--workdir=", 1, args);
 }
 
-static int set_cgroup_parent(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "cgroup-parent", "--cgroup-parent=", 1, out, outlen);
+static int set_cgroup_parent(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "cgroup-parent", "--cgroup-parent=", 1, args);
 }
 
-static int set_hostname(const struct configuration *command_config, char *out, const size_t outlen) {
-  return add_param_to_command(command_config, "hostname", "--hostname=", 1, out, outlen);
+static int set_hostname(const struct configuration *command_config, args *args) {
+  return add_param_to_command(command_config, "hostname", "--hostname=", 1, args);
 }
 
-static int set_group_add(const struct configuration *command_config, char *out, const size_t outlen) {
+static int set_group_add(const struct configuration *command_config, args *args) {
   int i = 0, ret = 0;
   char **group_add = get_configuration_values_delimiter("group-add", DOCKER_COMMAND_FILE_SECTION, command_config, ",");
-  size_t tmp_buffer_size = 4096;
-  char *tmp_buffer = NULL;
   char *privileged = NULL;
 
   privileged = get_configuration_value("privileged", DOCKER_COMMAND_FILE_SECTION, command_config);
   if (privileged != NULL && strcasecmp(privileged, "true") == 0 ) {
-    free(privileged);
-    return ret;
+    goto free_and_exit;
   }
-  free(privileged);
 
   if (group_add != NULL) {
     for (i = 0; group_add[i] != NULL; ++i) {
-      tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
-      quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "--group-add ", group_add[i]);
-      ret = add_to_buffer(out, outlen, tmp_buffer);
+      ret = add_to_args(args, "--group-add");
       if (ret != 0) {
-        return BUFFER_TOO_SMALL;
+        goto free_and_exit;
+      }
+      ret = add_to_args(args, group_add[i]);
+      if (ret != 0) {
+        goto free_and_exit;
       }
     }
   }
+free_and_exit:
+  free_values(group_add);
+  free(privileged);
   return ret;
 }
 
 static int set_network(const struct configuration *command_config,
-                       const struct configuration *conf, char *out,
-                       const size_t outlen) {
+                       const struct configuration *conf, args *args) {
 
   int ret = 0;
   ret = add_param_to_command_if_allowed(command_config, conf, "net",
                                         "docker.allowed.networks", "--net=",
-                                        0, 0, out, outlen);
+                                        0, 0, args);
   if (ret != 0) {
     fprintf(ERRORFILE, "Could not find requested network in allowed networks\n");
     ret = INVALID_DOCKER_NETWORK;
-    memset(out, 0, outlen);
   }
 
   return ret;
 }
 
 static int set_pid_namespace(const struct configuration *command_config,
-                   const struct configuration *conf, char *out,
-                   const size_t outlen) {
-  size_t tmp_buffer_size = 1024;
-  char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
+                   const struct configuration *conf, args *args) {
   char *value = get_configuration_value("pid", DOCKER_COMMAND_FILE_SECTION,
       command_config);
   char *pid_host_enabled = get_configuration_value("docker.host-pid-namespace.enabled",
@@ -940,7 +933,7 @@ static int set_pid_namespace(const struct configuration *command_config,
       if (pid_host_enabled != NULL) {
         if (strcmp(pid_host_enabled, "1") == 0 ||
             strcasecmp(pid_host_enabled, "True") == 0) {
-          ret = add_to_buffer(out, outlen, "--pid='host' ");
+          ret = add_to_args(args, "--pid='host'");
           if (ret != 0) {
             ret = BUFFER_TOO_SMALL;
           }
@@ -960,23 +953,17 @@ static int set_pid_namespace(const struct configuration *command_config,
     }
   }
 
-  free_and_exit:
-  free(tmp_buffer);
+free_and_exit:
   free(value);
   free(pid_host_enabled);
-  if (ret != 0) {
-    memset(out, 0, outlen);
-  }
   return ret;
 }
 
 static int set_capabilities(const struct configuration *command_config,
-                            const struct configuration *conf, char *out,
-                            const size_t outlen) {
-
+                            const struct configuration *conf, args *args) {
   int ret = 0;
 
-  ret = add_to_buffer(out, outlen, "--cap-drop='ALL' ");
+  ret = add_to_args(args, "--cap-drop=ALL");
   if (ret != 0) {
     return BUFFER_TOO_SMALL;
   }
@@ -984,7 +971,7 @@ static int set_capabilities(const struct configuration *command_config,
   ret = add_param_to_command_if_allowed(command_config, conf, "cap-add",
                                         "docker.allowed.capabilities",
                                         "--cap-add=", 1, 0,
-                                        out, outlen);
+                                        args);
   switch (ret) {
     case 0:
       break;
@@ -995,21 +982,18 @@ static int set_capabilities(const struct configuration *command_config,
     default:
       fprintf(ERRORFILE, "Invalid docker capability requested\n");
       ret = INVALID_DOCKER_CAPABILITY;
-      memset(out, 0, outlen);
   }
 
   return ret;
 }
 
-static int set_devices(const struct configuration *command_config, const struct configuration *conf, char *out,
-                       const size_t outlen) {
+static int set_devices(const struct configuration *command_config, const struct configuration *conf, args *args) {
   int ret = 0;
   ret = add_param_to_command_if_allowed(command_config, conf, "devices", "docker.allowed.devices", "--device=", 1, ':',
-                                        out, outlen);
+                                        args);
   if (ret != 0) {
     fprintf(ERRORFILE, "Invalid docker device requested\n");
     ret = INVALID_DOCKER_DEVICE;
-    memset(out, 0, outlen);
   }
 
   return ret;
@@ -1046,7 +1030,6 @@ static char* normalize_mount(const char* mount, int isRegexAllowed) {
       }
     }
     fprintf(ERRORFILE, "Could not determine real path of mount '%s'\n", mount);
-    free(real_mount);
     return NULL;
   }
   ret = stat(real_mount, &buff);
@@ -1054,6 +1037,7 @@ static char* normalize_mount(const char* mount, int isRegexAllowed) {
     if (S_ISDIR(buff.st_mode)) {
       size_t len = strlen(real_mount);
       if (len <= 0) {
+        free(real_mount);
         return NULL;
       }
       if (real_mount[len - 1] != '/') {
@@ -1095,10 +1079,10 @@ static int normalize_mounts(char **mounts, int isRegexAllowed) {
 static int check_mount_permitted(const char **permitted_mounts, const char *requested) {
   int i = 0, ret = 0;
   size_t permitted_mount_len = 0;
-  char *normalized_path = normalize_mount(requested, 0);
   if (permitted_mounts == NULL) {
     return 0;
   }
+  char *normalized_path = normalize_mount(requested, 0);
   if (normalized_path == NULL) {
     return -1;
   }
@@ -1124,36 +1108,31 @@ static int check_mount_permitted(const char **permitted_mounts, const char *requ
         break;
       }
     }
-
   }
   free(normalized_path);
   return ret;
 }
 
 static char* get_mount_source(const char *mount) {
-  char *src_mount = NULL;
-  const char *tmp = NULL;
-  tmp = strchr(mount, ':');
+  const char *tmp = strchr(mount, ':');
   if (tmp == NULL) {
     fprintf(ERRORFILE, "Invalid docker mount '%s'\n", mount);
     return NULL;
   }
-  src_mount = strndup(mount, tmp - mount);
-  return src_mount;
+  size_t len = tmp - mount;
+  return strndup(mount, len);
 }
 
 static int add_mounts(const struct configuration *command_config, const struct configuration *conf, const char *key,
-                      const int ro, char *out, const size_t outlen) {
-  size_t tmp_buffer_size = 1024;
+                      const int ro, args *args) {
   const char *ro_suffix = "";
   const char *tmp_path_buffer[2] = {NULL, NULL};
-  char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
+  char *mount_src = NULL;
   char **permitted_ro_mounts = get_configuration_values_delimiter("docker.allowed.ro-mounts",
                                                                   CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
   char **permitted_rw_mounts = get_configuration_values_delimiter("docker.allowed.rw-mounts",
                                                                   CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf, ",");
   char **values = get_configuration_values_delimiter(key, DOCKER_COMMAND_FILE_SECTION, command_config, ",");
-  char *tmp_buffer_2 = NULL, *mount_src = NULL;
   const char *container_executor_cfg_path = normalize_mount(get_config_path(""), 0);
   int i = 0, permitted_rw = 0, permitted_ro = 0, ret = 0;
   if (ro != 0) {
@@ -1180,10 +1159,10 @@ static int add_mounts(const struct configuration *command_config, const struct c
       ret = MOUNT_ACCESS_ERROR;
       goto free_and_exit;
     }
-    for (i = 0; values[i] != NULL; ++i) {
+    for (i = 0; values[i] != NULL; i++) {
       mount_src = get_mount_source(values[i]);
       if (mount_src == NULL) {
-        fprintf(ERRORFILE, "Invalid docker mount '%s', realpath=%s\n", values[i], mount_src);
+        fprintf(ERRORFILE, "Invalid docker mount '%s'\n", values[i]);
         ret = INVALID_DOCKER_MOUNT;
         goto free_and_exit;
       }
@@ -1221,16 +1200,16 @@ static int add_mounts(const struct configuration *command_config, const struct c
         ret = INVALID_DOCKER_RO_MOUNT;
         goto free_and_exit;
       }
-      tmp_buffer_2 = (char *) alloc_and_clear_memory(strlen(values[i]) + strlen(ro_suffix) + 1, sizeof(char));
-      strncpy(tmp_buffer_2, values[i], strlen(values[i]));
-      strncpy(tmp_buffer_2 + strlen(values[i]), ro_suffix, strlen(ro_suffix));
-      quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "-v ", tmp_buffer_2);
-      ret = add_to_buffer(out, outlen, tmp_buffer);
-      free(tmp_buffer_2);
-      free(mount_src);
-      tmp_buffer_2 = NULL;
-      mount_src = NULL;
-      memset(tmp_buffer, 0, tmp_buffer_size);
+
+      ret = add_to_args(args, "-v");
+      if (ret != 0) {
+        ret = BUFFER_TOO_SMALL;
+        goto free_and_exit;
+      }
+
+      char *tmp_buffer = make_string("%s%s", values[i], (char *) ro_suffix);
+      ret = add_to_args(args, tmp_buffer);
+      free(tmp_buffer);
       if (ret != 0) {
         ret = BUFFER_TOO_SMALL;
         goto free_and_exit;
@@ -1238,27 +1217,21 @@ static int add_mounts(const struct configuration *command_config, const struct c
     }
   }
 
-  free_and_exit:
+free_and_exit:
+  free(mount_src);
   free_values(permitted_ro_mounts);
   free_values(permitted_rw_mounts);
   free_values(values);
-  free(mount_src);
   free((void *) container_executor_cfg_path);
-  free(tmp_buffer);
-  if (ret != 0) {
-    memset(out, 0, outlen);
-  }
   return ret;
 }
 
-static int add_ro_mounts(const struct configuration *command_config, const struct configuration *conf, char *out,
-                          const size_t outlen) {
-  return add_mounts(command_config, conf, "ro-mounts", 1, out, outlen);
+static int add_ro_mounts(const struct configuration *command_config, const struct configuration *conf, args *args) {
+  return add_mounts(command_config, conf, "ro-mounts", 1, args);
 }
 
-static int  add_rw_mounts(const struct configuration *command_config, const struct configuration *conf, char *out,
-                          const size_t outlen) {
-  return add_mounts(command_config, conf, "rw-mounts", 0, out, outlen);
+static int  add_rw_mounts(const struct configuration *command_config, const struct configuration *conf, args *args) {
+  return add_mounts(command_config, conf, "rw-mounts", 0, args);
 }
 
 static int check_privileges(const char *user) {
@@ -1334,20 +1307,18 @@ static int check_privileges(const char *user) {
   return ret;
 }
 
-static int set_privileged(const struct configuration *command_config, const struct configuration *conf, char *out,
-                          const size_t outlen) {
-  size_t tmp_buffer_size = 1024;
+static int set_privileged(const struct configuration *command_config, const struct configuration *conf, args *args) {
   char *user = NULL;
-  char *tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
   char *value = get_configuration_value("privileged", DOCKER_COMMAND_FILE_SECTION, command_config);
   char *privileged_container_enabled
       = get_configuration_value("docker.privileged-containers.enabled", CONTAINER_EXECUTOR_CFG_DOCKER_SECTION, conf);
   int ret = 0;
-  int allowed = 0;
+  int allowed = 1;
 
   user = get_configuration_value("user", DOCKER_COMMAND_FILE_SECTION, command_config);
   if (user == NULL) {
-    return INVALID_DOCKER_USER_NAME;
+    ret = INVALID_DOCKER_USER_NAME;
+    goto free_and_exit;
   }
 
   if (value != NULL && strcasecmp(value, "true") == 0 ) {
@@ -1362,7 +1333,7 @@ static int set_privileged(const struct configuration *command_config, const stru
         }
         allowed = check_privileges(user);
         if (allowed) {
-          ret = add_to_buffer(out, outlen, "--privileged ");
+          ret = add_to_args(args, "--privileged");
           if (ret != 0) {
             ret = BUFFER_TOO_SMALL;
           }
@@ -1384,20 +1355,15 @@ static int set_privileged(const struct configuration *command_config, const stru
   }
 
   free_and_exit:
-  free(tmp_buffer);
   free(value);
   free(privileged_container_enabled);
   free(user);
-  if (ret != 0) {
-    memset(out, 0, outlen);
-  }
   return ret;
 }
 
-int get_docker_run_command(const char *command_file, const struct configuration *conf, char *out, const size_t outlen) {
+int get_docker_run_command(const char *command_file, const struct configuration *conf, args *args) {
   int ret = 0, i = 0;
   char *container_name = NULL, *user = NULL, *image = NULL;
-  size_t tmp_buffer_size = 1024;
   char *tmp_buffer = NULL;
   char **launch_command = NULL;
   char *privileged = NULL;
@@ -1409,6 +1375,9 @@ int get_docker_run_command(const char *command_file, const struct configuration
 
   container_name = get_configuration_value("name", DOCKER_COMMAND_FILE_SECTION, &command_config);
   if (container_name == NULL || validate_container_name(container_name) != 0) {
+    if (container_name != NULL) {
+      free(container_name);
+    }
     return INVALID_DOCKER_CONTAINER_NAME;
   }
   user = get_configuration_value("user", DOCKER_COMMAND_FILE_SECTION, &command_config);
@@ -1417,127 +1386,148 @@ int get_docker_run_command(const char *command_file, const struct configuration
   }
   image = get_configuration_value("image", DOCKER_COMMAND_FILE_SECTION, &command_config);
   if (image == NULL || validate_docker_image_name(image) != 0) {
+    if (image != NULL) {
+      free(image);
+    }
     return INVALID_DOCKER_IMAGE_NAME;
   }
 
-  ret = add_docker_config_param(&command_config, out, outlen);
+  char *docker = get_docker_binary(conf);
+  ret = add_to_args(args, docker);
+  free(docker);
   if (ret != 0) {
+    reset_args(args);
     return BUFFER_TOO_SMALL;
   }
 
-  ret = add_to_buffer(out, outlen, DOCKER_RUN_COMMAND);
-  if(ret != 0) {
+  ret = add_docker_config_param(&command_config, args);
+  if (ret != 0) {
+    reset_args(args);
     return BUFFER_TOO_SMALL;
   }
 
+  ret = add_to_args(args, DOCKER_RUN_COMMAND);
+  if(ret != 0) {
+    reset_args(args);
+    return BUFFER_TOO_SMALL;
+  }
 
-  tmp_buffer = (char *) alloc_and_clear_memory(tmp_buffer_size, sizeof(char));
-
-  quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, " --name=", container_name);
-  ret = add_to_buffer(out, outlen, tmp_buffer);
+  tmp_buffer = make_string("--name=%s", container_name);
+  ret = add_to_args(args, tmp_buffer);
   if (ret != 0) {
+    reset_args(args);
     return BUFFER_TOO_SMALL;
   }
-  memset(tmp_buffer, 0, tmp_buffer_size);
 
   privileged = get_configuration_value("privileged", DOCKER_COMMAND_FILE_SECTION, &command_config);
 
-  if (privileged == NULL || strcasecmp(privileged, "false") == 0) {
-      quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "--user=", user);
-      ret = add_to_buffer(out, outlen, tmp_buffer);
-      if (ret != 0) {
-        return BUFFER_TOO_SMALL;
-      }
-      memset(tmp_buffer, 0, tmp_buffer_size);
+  if (privileged == NULL || strcmp(privileged, "false") == 0) {
+    char *user_buffer = make_string("--user=%s", user);
+    ret = add_to_args(args, user_buffer);
+    free(user_buffer);
+    if (ret != 0) {
+      reset_args(args);
+      return BUFFER_TOO_SMALL;
+    }
   }
   free(privileged);
 
-  ret = detach_container(&command_config, out, outlen);
+  ret = detach_container(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = rm_container_on_exit(&command_config, out, outlen);
+  ret = rm_container_on_exit(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_container_workdir(&command_config, out, outlen);
+  ret = set_container_workdir(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_network(&command_config, conf, out, outlen);
+  ret = set_network(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_pid_namespace(&command_config, conf, out, outlen);
+  ret = set_pid_namespace(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = add_ro_mounts(&command_config, conf, out, outlen);
+  ret = add_ro_mounts(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = add_rw_mounts(&command_config, conf, out, outlen);
+  ret = add_rw_mounts(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_cgroup_parent(&command_config, out, outlen);
+  ret = set_cgroup_parent(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_privileged(&command_config, conf, out, outlen);
+  ret = set_privileged(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_capabilities(&command_config, conf, out, outlen);
+  ret = set_capabilities(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_hostname(&command_config, out, outlen);
+  ret = set_hostname(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_group_add(&command_config, out, outlen);
+  ret = set_group_add(&command_config, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  ret = set_devices(&command_config, conf, out, outlen);
+  ret = set_devices(&command_config, conf, args);
   if (ret != 0) {
+    reset_args(args);
     return ret;
   }
 
-  quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "", image);
-  ret = add_to_buffer(out, outlen, tmp_buffer);
+  ret = add_to_args(args, image);
   if (ret != 0) {
+    reset_args(args);
     return BUFFER_TOO_SMALL;
   }
 
   launch_command = get_configuration_values_delimiter("launch-command", DOCKER_COMMAND_FILE_SECTION, &command_config,
                                                       ",");
-
   if (check_trusted_image(&command_config, conf) != 0) {
     launch_command = NULL;
   }
 
   if (launch_command != NULL) {
     for (i = 0; launch_command[i] != NULL; ++i) {
-      memset(tmp_buffer, 0, tmp_buffer_size);
-      quote_and_append_arg(&tmp_buffer, &tmp_buffer_size, "", launch_command[i]);
-      ret = add_to_buffer(out, outlen, tmp_buffer);
+      ret = add_to_args(args, launch_command[i]);
       if (ret != 0) {
         free_values(launch_command);
-        free(tmp_buffer);
+        reset_args(args);
         return BUFFER_TOO_SMALL;
       }
     }
@@ -1546,6 +1536,3 @@ int get_docker_run_command(const char *command_file, const struct configuration
   free(tmp_buffer);
   return 0;
 }
-
-
-

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h
index c797ecd..330d722 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/docker-util.h
@@ -33,7 +33,13 @@
 #define DOCKER_KILL_COMMAND "kill"
 #define DOCKER_VOLUME_COMMAND "volume"
 #define DOCKER_START_COMMAND "start"
+#define DOCKER_ARG_MAX 1024
+#define ARGS_INITIAL_VALUE { 0 };
 
+typedef struct args {
+    int length;
+    char *data[DOCKER_ARG_MAX];
+} args;
 
 enum docker_error_codes {
     INVALID_COMMAND_FILE = 1,
@@ -77,7 +83,7 @@ char *get_docker_binary(const struct configuration *conf);
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker inspect command line string. The function will verify that the params file is meant for the
@@ -88,7 +94,7 @@ int get_docker_command(const char* command_file, const struct configuration* con
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_inspect_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_inspect_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker load command line string. The function will verify that the params file is meant for the load command.
@@ -98,7 +104,7 @@ int get_docker_inspect_command(const char* command_file, const struct configurat
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_load_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_load_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker pull command line string. The function will verify that the params file is meant for the pull command.
@@ -108,7 +114,7 @@ int get_docker_load_command(const char* command_file, const struct configuration
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_pull_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_pull_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker rm command line string. The function will verify that the params file is meant for the rm command.
@@ -118,7 +124,7 @@ int get_docker_pull_command(const char* command_file, const struct configuration
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_rm_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_rm_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker run command line string. The function will verify that the params file is meant for the run command.
@@ -128,7 +134,7 @@ int get_docker_rm_command(const char* command_file, const struct configuration*
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_run_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_run_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker stop command line string. The function will verify that the params file is meant for the stop command.
@@ -138,7 +144,7 @@ int get_docker_run_command(const char* command_file, const struct configuration*
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_stop_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_stop_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker kill command line string. The function will verify that the params file is meant for the kill command.
@@ -148,7 +154,7 @@ int get_docker_stop_command(const char* command_file, const struct configuration
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_kill_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_kill_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Get the Docker volume command line string. The function will verify that the
@@ -159,8 +165,7 @@ int get_docker_kill_command(const char* command_file, const struct configuration
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_volume_command(const char *command_file, const struct configuration *conf, char *out,
-                               const size_t outlen);
+int get_docker_volume_command(const char *command_file, const struct configuration *conf, args *args);
 
 /**
  * Get the Docker start command line string. The function will verify that the params file is meant for the start command.
@@ -170,7 +175,7 @@ int get_docker_volume_command(const char *command_file, const struct configurati
  * @param outlen Size of the output buffer
  * @return Return code with 0 indicating success and non-zero codes indicating error
  */
-int get_docker_start_command(const char* command_file, const struct configuration* conf, char *out, const size_t outlen);
+int get_docker_start_command(const char* command_file, const struct configuration* conf, args *args);
 
 /**
  * Give an error message for the supplied error code
@@ -186,4 +191,15 @@ const char *get_docker_error_message(const int error_code);
  */
 int docker_module_enabled(const struct configuration *conf);
 
+/**
+ * Helper function to reset args data structure.
+ * @param args Pointer reference to args data structure
+ */
+void reset_args(args *args);
+
+/**
+ * Extract execv args from args data structure.
+ * @param args Pointer reference to args data structure
+ */
+char** extract_execv_args(args *args);
 #endif

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c
index 40b2c25..80511e5 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.c
@@ -22,6 +22,7 @@
 #include <strings.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stdarg.h>
 
 /*
  * if all chars in the input str are numbers
@@ -156,3 +157,26 @@ cleanup:
   }
   return is_container_id;
 }
+
+/*
+ * Format string utility.
+ */
+char *make_string(const char *fmt, ...) {
+  va_list vargs;
+  va_start(vargs, fmt);
+  size_t buflen = vsnprintf(NULL, 0, fmt, vargs) + 1;
+  va_end(vargs);
+  if (buflen <= 0) {
+    return NULL;
+  }
+  char* buf = malloc(buflen);
+  if (buf != NULL) {
+    va_start(vargs, fmt);
+    int ret = vsnprintf(buf, buflen, fmt, vargs);
+    va_end(vargs);
+    if (ret < 0) {
+      buf = NULL;
+    }
+  }
+  return buf;
+}

http://git-wip-us.apache.org/repos/asf/hadoop/blob/a2ea7564/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h
----------------------------------------------------------------------
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h
index c095eb6..affb3c3 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/utils/string-utils.h
@@ -34,4 +34,8 @@ int validate_container_id(const char* input);
  */
 int get_numbers_split_by_comma(const char* input, int** numbers, size_t* n_numbers);
 
+/*
+ * String format utility
+ */
+char *make_string(const char *fmt, ...);
 #endif


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org