Change key-value API to Colin's recommendation

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c

@@ -325,39 +325,66 @@ void free_values(char** values) {
 }
 
 /**
- * Returns the key in a key,value pair of the form key=value
+ * If str is a string of the form key=val, find 'key'
+ * 
+ * @param input    The input string
+ * @param out      Where to put the output string.
+ * @param out_len  The length of the output buffer.
+ *
+ * @return         -ENAMETOOLONG if out_len is not long enough;
+ *                 -EINVAL if there is no equals sign in the input;
+ *                 0 on success
  */
-char *get_kv_key(char *pair) {
-  char *toPass = pair;
-  char *equaltok;
-  char *temp_equaltok;
+int get_kv_key(const char *input, char *out, size_t out_len) {
 
-  if (pair != NULL) {
-     equaltok = strtok_r(pair, "=", &temp_equaltok);
-     if (equaltok != NULL) {
-       toPass = equaltok;
-     }
-  }
+  if (input == NULL)
+    return -EINVAL;
 
-  return toPass;
+  char *split = strchr(input, '=');
+
+  if (split == NULL)
+    return -EINVAL;
+
+  int key_len = split - input;
+
+  if (out_len < (key_len + 1) || out == NULL)
+    return -ENAMETOOLONG;
+
+  memcpy(out, input, key_len);
+  out[key_len] = '\0';
+
+  return 0;
 }
 
 /**
- * Returns the value in a key,value pair of the form key=value
+ * If str is a string of the form key=val, find 'val'
+ * 
+ * @param input    The input string
+ * @param out      Where to put the output string.
+ * @param out_len  The length of the output buffer.
+ *
+ * @return         -ENAMETOOLONG if out_len is not long enough;
+ *                 -EINVAL if there is no equals sign in the input;
+ *                 0 on success
  */
-char *get_kv_value(char *pair) {
-  char *toPass = pair;
-  char *equaltok;
-  char *temp_equaltok;
+int get_kv_value(const char *input, char *out, size_t out_len) {
 
-  if (pair != NULL) {
-     equaltok = strtok_r(pair, "=", &temp_equaltok);
-     if (equaltok != NULL) {
-       toPass = temp_equaltok;
-     } else {
-       toPass = NULL;
-     }
-  }
+  if (input == NULL)
+    return -EINVAL;
 
-  return toPass;
+  char *split = strchr(input, '=');
+
+  if (split == NULL)
+    return -EINVAL;
+
+  split++; // advance past '=' to the value
+  int val_len = (input + strlen(input)) - split;
+
+  if (out_len < (val_len + 1) || out == NULL)
+    return -ENAMETOOLONG;
+
+  memcpy(out, split, val_len);
+  out[val_len] = '\0';
+
+  return 0;
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h

@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+#include <stddef.h>
+
 /**
  * Ensure that the configuration file and all of the containing directories
  * are only writable by root. Otherwise, an attacker can change the 
@@ -51,5 +53,5 @@ void free_values(char** values);
 void free_configurations();
 
 // methods to return the key or value in a key=value pair
-char *get_kv_key(char *pair);
-char *get_kv_value(char *pair);
+int get_kv_key(const char *input, char *out, size_t out_len);
+int get_kv_value(const char *input, char *out, size_t out_len);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/container-executor.c

@@ -1194,14 +1194,17 @@ void chown_dir_contents(const char *dir_path, uid_t uid, gid_t gid) {
  * hierarchy: the top directory of the hierarchy for the NM
  */
 int mount_cgroup(const char *pair, const char *hierarchy) {
-  char *for_key = strdup(pair);
-  char* for_value = strdup(pair);
-  char *controller = get_kv_key(for_key);
-  char *mount_path = get_kv_value(for_value);
+  char *controller = malloc(strlen(pair));
+  char *mount_path = malloc(strlen(pair));
   char hier_path[PATH_MAX];
   int result = 0;
 
-  if (mount("none", mount_path, "cgroup", 0, controller) == 0) {
+  if (get_kv_key(pair, controller, strlen(pair)) < 0 ||
+      get_kv_value(pair, mount_path, strlen(pair)) < 0) {
+    result = -1; 
+  }
+
+  if (result >= 0 && mount("none", mount_path, "cgroup", 0, controller) == 0) {
     char *buf = stpncpy(hier_path, mount_path, strlen(mount_path));
     *buf++ = '/';
     snprintf(buf, sizeof(buf), "%s", hierarchy);
@@ -1218,8 +1221,8 @@ int mount_cgroup(const char *pair, const char *hierarchy) {
     result = -1;
   }
 
-  free(for_key);
-  free(for_value);
+  free(controller);
+  free(mount_path);
 
   return result;
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c

@@ -218,17 +218,25 @@ int main(int argc, char **argv) {
     local_dirs = argv[optind++];// good local dirs as a comma separated list
     log_dirs = argv[optind++];// good log dirs as a comma separated list
     resources = argv[optind++];// key,value pair describing resources
-    char *for_key = strdup(resources);
-    char *for_value = strdup(resources);
-    resources_key = get_kv_key(for_key);
-    resources_value = extract_values(get_kv_value(for_value));
+    char *resources_key = malloc(strlen(resources));
+    char *resources_value = malloc(strlen(resources));
+    if (get_kv_key(resources, resources_key, strlen(resources)) < 0 ||
+        get_kv_value(resources, resources_value, strlen(resources)) < 0) {
+        fprintf(ERRORFILE, "Invalid arguments for cgroups resources: %s",
+                           resources);
+        fflush(ERRORFILE);
+        free(resources_key);
+        free(resources_value);
+        return INVALID_ARGUMENT_NUMBER;
+    }
+    char** resources_values = extract_values(resources_value);
     exit_code = launch_container_as_user(user_detail->pw_name, app_id,
                     container_id, current_dir, script_file, cred_file,
                     pid_file, extract_values(local_dirs),
                     extract_values(log_dirs), resources_key,
-                    resources_value);
-    free(for_key);
-    free(for_value);
+                    resources_values);
+    free(resources_key);
+    free(resources_value);
     break;
   case SIGNAL_CONTAINER:
     if (argc != 5) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c

@@ -611,11 +611,17 @@ void test_run_container() {
 	   strerror(errno));
     exit(1);
   } else if (child == 0) {
+    char *key = malloc(strlen(resources));
+    char *value = malloc(strlen(resources));
+    if (get_kv_key(resources, key, strlen(resources)) < 0 ||
+        get_kv_value(resources, key, strlen(resources)) < 0) {
+        printf("FAIL: resources failed - %s\n");
+        exit(1);
+    }
     if (launch_container_as_user(username, "app_4", "container_1", 
           container_dir, script_name, TEST_ROOT "/creds.txt", pid_file,
           extract_values(local_dirs), extract_values(log_dirs),
-          get_kv_key(resources),
-          extract_values(get_kv_value(resources))) != 0) {
+          key, extract_values(value)) != 0) {
       printf("FAIL: failed in child\n");
       exit(42);
     }