Skip to content

Commit a339cb4

Browse files
slpalyssarosenzweig
authored andcommitted
Pass args and envs using a config file
When using "krun_set_exec", the arguments and environment variables are passed to the guest using the kernel command line. In addition to being limited in length, this is also only compatible with ASCII strings. libkrun is also capable or reading both arguments and environment variables from a JSON config file, which doesn't suffer from any of those limitations, so switch to using this approach instead. Depends on containers/libkrun#220 Signed-off-by: Sergio Lopez <[email protected]>
1 parent 7e57408 commit a339cb4

2 files changed

Lines changed: 62 additions & 72 deletions

File tree

crates/krun/src/bin/krun.rs

Lines changed: 58 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::ffi::{c_char, CString};
2+
use std::io::Write;
23
use std::os::fd::{IntoRawFd, OwnedFd};
34
use std::path::Path;
45
use std::{cmp, env};
@@ -11,7 +12,7 @@ use krun::launch::{launch_or_lock, LaunchResult};
1112
use krun::net::{connect_to_passt, start_passt};
1213
use krun::types::MiB;
1314
use krun_sys::{
14-
krun_add_disk, krun_add_vsock_port, krun_create_ctx, krun_set_exec, krun_set_gpu_options,
15+
krun_add_disk, krun_add_vsock_port, krun_create_ctx, krun_set_env, krun_set_gpu_options,
1516
krun_set_log_level, krun_set_passt_fd, krun_set_root, krun_set_vm_config, krun_set_workdir,
1617
krun_start_enter, VIRGLRENDERER_DRM, VIRGLRENDERER_THREAD_SYNC,
1718
VIRGLRENDERER_USE_ASYNC_FENCE_CB, VIRGLRENDERER_USE_EGL,
@@ -23,6 +24,21 @@ use rustix::io::Errno;
2324
use rustix::process::{
2425
geteuid, getgid, getrlimit, getuid, sched_setaffinity, setrlimit, CpuSet, Resource,
2526
};
27+
use serde::{Deserialize, Serialize};
28+
use tempfile::NamedTempFile;
29+
30+
#[derive(Serialize, Deserialize)]
31+
pub struct KrunConfig {
32+
#[serde(rename = "Cmd")]
33+
args: Vec<String>,
34+
#[serde(rename = "Env")]
35+
envs: Vec<String>,
36+
}
37+
#[derive(Serialize, Deserialize)]
38+
pub struct KrunBaseConfig {
39+
#[serde(rename = "Config")]
40+
config: KrunConfig,
41+
}
2642

2743
fn add_ro_disk(ctx_id: u32, label: &str, path: &str) -> Result<()> {
2844
let path_cstr = CString::new(path).unwrap();
@@ -294,90 +310,66 @@ fn main() -> Result<()> {
294310
let krun_guest_path = find_krun_exec("krun-guest")?;
295311
let krun_server_path = find_krun_exec("krun-server")?;
296312

297-
let mut krun_guest_args: Vec<CString> = vec![
298-
CString::new(username).expect("username should not contain NUL character"),
299-
CString::new(format!("{}", getuid().as_raw()))
300-
.expect("uid should not contain NUL character"),
301-
CString::new(format!("{}", getgid().as_raw()))
302-
.expect("gid should not contain NUL character"),
313+
let mut krun_guest_args: Vec<String> = vec![
314+
krun_guest_path,
315+
username,
316+
format!("{uid}", uid = getuid().as_raw()),
317+
format!("{gid}", gid = getgid().as_raw()),
318+
krun_server_path,
319+
command
320+
.to_str()
321+
.context("Failed to process command as it contains invalid UTF-8")?
322+
.to_string(),
303323
];
304-
305-
krun_guest_args.push(krun_server_path);
306-
krun_guest_args.push(
307-
CString::new(
308-
command
309-
.to_str()
310-
.context("Failed to process command as it contains invalid UTF-8")?,
311-
)
312-
.context("Failed to process command as it contains NUL character")?,
313-
);
314-
let command_argc = command_args.len();
315324
for arg in command_args {
316-
let s = CString::new(arg)
317-
.context("Failed to process command arg as it contains NUL character")?;
318-
krun_guest_args.push(s);
325+
krun_guest_args.push(arg);
319326
}
320327

321-
let krun_guest_args: Vec<*const c_char> = {
322-
const KRUN_GUEST_ARGS_FIXED: usize = 4;
323-
// SAFETY: All pointers must be stored in the same allocation.
324-
// See https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html#safety
325-
let mut vec = Vec::with_capacity(KRUN_GUEST_ARGS_FIXED + command_argc + 1);
326-
for s in &krun_guest_args {
327-
vec.push(s.as_ptr());
328-
}
329-
vec.push(std::ptr::null());
330-
vec
331-
};
332-
333328
let mut env = prepare_env_vars(env).context("Failed to prepare environment variables")?;
334329
env.insert(
335330
"KRUN_SERVER_PORT".to_owned(),
336331
options.server_port.to_string(),
337332
);
338-
let env: Vec<CString> = {
339-
let mut vec = Vec::with_capacity(env.len());
340-
for (key, value) in env {
341-
let s = CString::new(format!("{key}={value}")).with_context(|| {
342-
format!("Failed to process `{key}` env var as it contains NUL character")
343-
})?;
344-
vec.push(s);
345-
}
346-
vec
333+
334+
let mut krun_config = KrunConfig {
335+
args: Vec::new(),
336+
envs: Vec::new(),
347337
};
348-
let env: Vec<*const c_char> = {
349-
// SAFETY: All pointers must be stored in the same allocation.
350-
// See https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html#safety
351-
let mut vec = Vec::with_capacity(env.len() + 1);
352-
for s in &env {
353-
vec.push(s.as_ptr());
354-
}
355-
vec.push(std::ptr::null());
356-
vec
338+
for arg in krun_guest_args {
339+
krun_config.args.push(arg);
340+
}
341+
for (key, value) in env {
342+
krun_config.envs.push(format!("{}={}", key, value));
343+
}
344+
let krun_config = KrunBaseConfig {
345+
config: krun_config,
357346
};
358347

348+
// SAFETY: `config_file` lifetime needs to exceed krun_start_enter's
349+
let mut config_file = NamedTempFile::new()
350+
.context("Failed to create a temporary file to store the guest config")?;
351+
write!(
352+
config_file,
353+
"{}",
354+
serde_json::to_string(&krun_config)
355+
.context("Failed to transform KrunConfig into a JSON string")?
356+
)
357+
.context("Failed to write to temporary config file")?;
358+
359+
let krun_config_env = CString::new(format!("KRUN_CONFIG={}", config_file.path().display()))
360+
.context("Failed to process config_file var as it contains NUL character")?;
361+
let env: Vec<*const c_char> = vec![krun_config_env.as_ptr(), std::ptr::null()];
362+
359363
{
360-
// Specify the path of the binary to be executed in the isolated context,
361-
// relative to the root path.
364+
// Sets environment variables to be configured in the context of the executable.
362365
//
363366
// SAFETY:
364-
// * `krun_guest_path` is a pointer to a `CString` with long enough lifetime.
365-
// * `krun_guest_args` is a pointer to a `Vec` of pointers to `CString`s all
366-
// with long enough lifetime.
367367
// * `env` is a pointer to a `Vec` of pointers to `CString`s all with long
368368
// enough lifetime.
369-
let err = unsafe {
370-
krun_set_exec(
371-
ctx_id,
372-
krun_guest_path.as_ptr(),
373-
krun_guest_args.as_ptr(),
374-
env.as_ptr(),
375-
)
376-
};
369+
let err = unsafe { krun_set_env(ctx_id, env.as_ptr()) };
377370
if err < 0 {
378371
let err = Errno::from_raw_os_error(-err);
379-
return Err(err)
380-
.context("Failed to configure the parameters for the executable to be run");
372+
return Err(err).context("Failed to set the environment variables in the guest");
381373
}
382374
}
383375

crates/krun/src/env.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::collections::HashMap;
22
use std::env::{self, VarError};
3-
use std::ffi::CString;
43
use std::fs;
54
use std::io::ErrorKind;
65
use std::path::Path;
@@ -83,7 +82,7 @@ pub fn prepare_env_vars(env: Vec<(String, Option<String>)>) -> Result<HashMap<St
8382
Ok(env_map)
8483
}
8584

86-
pub fn find_krun_exec<P>(program: P) -> Result<CString>
85+
pub fn find_krun_exec<P>(program: P) -> Result<String>
8786
where
8887
P: AsRef<Path>,
8988
{
@@ -97,10 +96,9 @@ where
9796
let path = path.context("Failed to get path of current running executable")?;
9897
path.with_file_name(program)
9998
};
100-
let path = CString::new(path.to_str().with_context(|| {
99+
let path = path.to_str().with_context(|| {
101100
format!("Failed to process {program:?} path as it contains invalid UTF-8")
102-
})?)
103-
.with_context(|| format!("Failed to process {program:?} path as it contains NUL character"))?;
101+
})?;
104102

105-
Ok(path)
103+
Ok(path.to_string())
106104
}

0 commit comments

Comments
 (0)