Merge pull request containers#6 from slp/args_as_array

libkrun: Make "args" an array of string pointers

examples/chroot_vm.c

@@ -12,13 +12,13 @@
 
 #define MAX_ARGS_LEN 4096
 
-int main(int argc, void **argv)
+int main(int argc, char *const argv[])
 {
-    // Use an empty NULL-terminated string as env_line, so libkrun won't inject the variables
-    // currently present in the environment into the microVM.
-    char env_line[] = "\0";
-    char args[MAX_ARGS_LEN] = "\0";
-    int args_len = 0;
+    char *const envp[] =
+    {
+        "TEST=works",
+        0
+    };
     int ctx_id;
     int err;
     int i;
@@ -59,25 +59,9 @@ int main(int argc, void **argv)
         return -1;
     }
 
-    // If we have additional arguments, collect them into "args".
-    for (i = 3; i < argc; i++) {
-        // We need to add an space as a separator.
-        int len = strlen(argv[i]) + 1;
-
-        if ((len + args_len) >= (MAX_ARGS_LEN - 1)) {
-            printf("Too many arguments\n");
-            return -1;
-        }
-
-        strncpy(&args[args_len], argv[i], MAX_ARGS_LEN - args_len - 1);
-        args_len += len;
-        args[args_len - 1] = ' ';
-    }
-    args[args_len] = '\0';
-
     // Use the second argument as the path of the binary to be executed in the isolated
     // context, relative to the root path.
-    if (err = krun_set_exec(ctx_id, argv[2], &args[0], &env_line[0])) {
+    if (err = krun_set_exec(ctx_id, argv[2], &argv[3], &envp[0])) {
         errno = -err;
         perror("Error configuring the parameters for the executable to be run");
         return -1;

include/libkrun.h

@@ -68,19 +68,18 @@ int32_t krun_set_root(uint32_t ctx_id, const char *root_path);
  * Arguments:
  *  "ctx_id"    - the configuration context ID.
  *  "exec_path" - the path to the executable, relative to the root configured with "krun_set_root".
- *  "args"      - a null-terminated string representing the arguments to be passed to the
- *                executable.
- *  "env_line"  - a null-terminated string with the variables to be injected into the context of
- *                the executable, with "KEY=VALUE" format. If NULL, it will auto-generate a line
- *                collecting the variables currently present in the environment.
+ *  "argv"      - an array of string pointers to be passed as arguments.
+ *  "envp"      - an array of string pointers to be injected as environment variables into the
+ *                context of the executable. If NULL, it will auto-generate an array collecting the
+ *                the variables currently present in the environment.
  *
  * Returns:
  *  Zero on success or a negative error number on failure.
  */
 int32_t krun_set_exec(uint32_t ctx_id,
                       const char *exec_path,
-                      const char *args,
-                      const char *env_line);
+                      char *const argv[],
+                      char *const envp[]);
 
 /*
  * Starts and enters the microVM with the configured parameters. The VMM will attempt to take over

src/libkrun/src/lib.rs

@@ -7,6 +7,7 @@ use std::convert::TryInto;
 use std::env;
 use std::ffi::CStr;
 use std::process;
+use std::slice;
 use std::sync::atomic::{AtomicI32, Ordering};
 use std::sync::Mutex;
 
@@ -25,6 +26,8 @@ use vmm::vmm_config::vsock::VsockDeviceConfig;
 const KRUNFW_MIN_VERSION: u32 = 1;
 // Value returned on success. We use libc's errors otherwise.
 const KRUN_SUCCESS: i32 = 0;
+// Maximum number of arguments/environment variables we allow
+const MAX_ARGS: usize = 4096;
 
 // Path to the init binary to be executed inside the VM.
 const INIT_PATH: &str = "/init.krun";
@@ -165,13 +168,28 @@ pub unsafe extern "C" fn krun_set_root(ctx_id: u32, c_root_path: *const c_char)
     KRUN_SUCCESS
 }
 
+unsafe fn collapse_str_array(array: &[*const c_char]) -> Result<String, std::str::Utf8Error> {
+    let mut strvec = Vec::new();
+
+    for item in array.iter().take(MAX_ARGS) {
+        if item.is_null() {
+            break;
+        } else {
+            let s = CStr::from_ptr(*item).to_str()?;
+            strvec.push(format!("\"{}\"", s));
+        }
+    }
+
+    Ok(strvec.join(" "))
+}
+
 #[allow(clippy::missing_safety_doc)]
 #[no_mangle]
 pub unsafe extern "C" fn krun_set_exec(
     ctx_id: u32,
     c_exec_path: *const c_char,
-    c_args: *const c_char,
-    c_env_line: *const c_char,
+    c_argv: *const *const c_char,
+    c_envp: *const *const c_char,
 ) -> i32 {
     let exec_path = match CStr::from_ptr(c_exec_path).to_str() {
         Ok(path) => path,
@@ -181,36 +199,38 @@ pub unsafe extern "C" fn krun_set_exec(
         }
     };
 
-    let args = if c_args.is_null() {
-        ""
-    } else {
-        match CStr::from_ptr(c_args).to_str() {
-            Ok(args) => args,
+    let args = if !c_argv.is_null() {
+        let argv_array: &[*const c_char] = slice::from_raw_parts(c_argv, MAX_ARGS);
+        match collapse_str_array(argv_array) {
+            Ok(s) => s,
             Err(e) => {
                 debug!("Error parsing args: {:?}", e);
                 return -libc::EINVAL;
             }
         }
+    } else {
+        "".to_string()
     };
 
-    let env_line = if c_env_line.is_null() {
-        env::vars()
-            .map(|(key, value)| format!(" {}={}", key, value))
-            .collect()
-    } else {
-        match CStr::from_ptr(c_env_line).to_str() {
-            Ok(env) => env.to_string(),
+    let env = if !c_envp.is_null() {
+        let envp_array: &[*const c_char] = slice::from_raw_parts(c_envp, MAX_ARGS);
+        match collapse_str_array(envp_array) {
+            Ok(s) => s,
             Err(e) => {
-                debug!("Error parsing env_line: {:?}", e);
+                debug!("Error parsing args: {:?}", e);
                 return -libc::EINVAL;
             }
         }
+    } else {
+        env::vars()
+            .map(|(key, value)| format!(" {}={}", key, value))
+            .collect()
     };
 
     let mut boot_source = BootSourceConfig::default();
     boot_source.kernel_cmdline_prolog = Some(format!(
         "{} init={} KRUN_INIT={} {}",
-        DEFAULT_KERNEL_CMDLINE, INIT_PATH, exec_path, env_line,
+        DEFAULT_KERNEL_CMDLINE, INIT_PATH, exec_path, env,
     ));
     boot_source.kernel_cmdline_epilog = Some(format!(" -- {}", args));