-
Notifications
You must be signed in to change notification settings - Fork 960
argv[0] in WASIX subprocesses should resolve to the atom rather than the full command #6516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1167,6 +1167,51 @@ impl WasiEnv { | |||||||||||||||||||||||||
| "Injected a command into the filesystem", | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Mount each atom under /bin/.__atoms/<pkg>/<atom> so that a | ||||||||||||||||||||||||||
| // process re-execing via argv[0] (set to that path) finds the raw | ||||||||||||||||||||||||||
| // wasm bytes without command metadata. We intentionally do NOT | ||||||||||||||||||||||||||
| // register these paths in bin_factory so they are loaded as plain | ||||||||||||||||||||||||||
| // wasm executables, bypassing prepare_spawn and preventing | ||||||||||||||||||||||||||
| // main_args from being re-injected. | ||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||
| // The path encodes the origin package so atoms with the same name | ||||||||||||||||||||||||||
| // from different packages are kept separate. An atom referenced by | ||||||||||||||||||||||||||
| // multiple commands is only written once (deduplicated by path). | ||||||||||||||||||||||||||
| let _ = root_fs.create_dir(Path::new("/bin/.__atoms")); | ||||||||||||||||||||||||||
| let mut mounted_atoms: std::collections::HashSet<String> = | ||||||||||||||||||||||||||
| std::collections::HashSet::new(); | ||||||||||||||||||||||||||
| for command in &pkg.commands { | ||||||||||||||||||||||||||
| if let Some(atom_path) = command.atom_vfs_path() { | ||||||||||||||||||||||||||
| if mounted_atoms.insert(atom_path.clone()) { | ||||||||||||||||||||||||||
| let atom = command.atom(); | ||||||||||||||||||||||||||
| // Create intermediate directories under /bin/.__atoms. | ||||||||||||||||||||||||||
| // The origin package segment may contain '/' (e.g. | ||||||||||||||||||||||||||
| // "namespace/package"), so we create each level. | ||||||||||||||||||||||||||
| if let Some(parent) = Path::new(&atom_path).parent() { | ||||||||||||||||||||||||||
| let mut partial = String::from("/bin/.__atoms"); | ||||||||||||||||||||||||||
| for component in parent | ||||||||||||||||||||||||||
| .strip_prefix("/bin/.__atoms") | ||||||||||||||||||||||||||
| .unwrap_or(Path::new("")) | ||||||||||||||||||||||||||
| .components() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| partial.push('/'); | ||||||||||||||||||||||||||
| partial | ||||||||||||||||||||||||||
| .push_str(component.as_os_str().to_str().unwrap_or_default()); | ||||||||||||||||||||||||||
| let _ = root_fs.create_dir(Path::new(&partial)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| let _ = write_readonly_buffer_to_fs(root_fs, Path::new(&atom_path), &atom) | ||||||||||||||||||||||||||
| .await; | ||||||||||||||||||||||||||
|
Arshia001 marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||
| tracing::debug!( | ||||||||||||||||||||||||||
| package=%pkg.id, | ||||||||||||||||||||||||||
| origin_package=%command.origin_package(), | ||||||||||||||||||||||||||
| atom_path=%atom_path, | ||||||||||||||||||||||||||
| "Injected atom into the filesystem", | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Arshia001 marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||
|
|
@@ -1342,8 +1387,13 @@ impl WasiEnv { | |||||||||||||||||||||||||
| args.splice(1..1, main_args); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if let Some(exec_name) = exec_name { | ||||||||||||||||||||||||||
| self.state.args.lock().unwrap()[0] = exec_name; | ||||||||||||||||||||||||||
| // Prefer an explicit exec_name from the annotation; otherwise use | ||||||||||||||||||||||||||
| // the atom's canonical VFS path (e.g. | ||||||||||||||||||||||||||
| // "/bin/.__atoms/wasmer/php/php") so that re-exec via argv[0] finds | ||||||||||||||||||||||||||
| // the raw wasm without triggering prepare_spawn again. | ||||||||||||||||||||||||||
| let argv0 = exec_name.or_else(|| cmd.atom_vfs_path()); | ||||||||||||||||||||||||||
| if let Some(argv0) = argv0 { | ||||||||||||||||||||||||||
|
Comment on lines
+1389
to
+1394
|
||||||||||||||||||||||||||
| // Prefer an explicit exec_name from the annotation; otherwise use | |
| // the atom's canonical VFS path (e.g. | |
| // "/bin/.__atoms/wasmer/php/php") so that re-exec via argv[0] finds | |
| // the raw wasm without triggering prepare_spawn again. | |
| let argv0 = exec_name.or_else(|| cmd.atom_vfs_path()); | |
| if let Some(argv0) = argv0 { | |
| // Only replace argv[0] when the package explicitly provides an | |
| // exec_name. Falling back to the atom VFS path here is unsafe | |
| // unless injection at that path is known to have succeeded, and a | |
| // missing path can break programs that re-exec themselves via | |
| // argv[0]. | |
| if let Some(argv0) = exec_name { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| use std::sync::Arc; | ||
|
|
||
| use tempfile::TempDir; | ||
| use wasmer_wasix::{ | ||
| PluggableRuntime, Runtime, | ||
| bin_factory::BinaryPackage, | ||
| runners::wasi::{RuntimeOrEngine, WasiRunner}, | ||
| runtime::{ | ||
| module_cache::{FileSystemCache, ModuleCache, SharedCache}, | ||
| package_loader::BuiltinPackageLoader, | ||
| task_manager::tokio::TokioTaskManager, | ||
| }, | ||
| }; | ||
|
|
||
| use super::run_build_script; | ||
|
|
||
| /// Verify that a process re-exec'd via argv[0] does not inherit the command's | ||
| /// main_args a second time. The C program re-execs itself with a "child" | ||
| /// marker; in child mode it asserts argc == 2 (only argv[0] and the marker). | ||
| /// Before the fix, the command's main_args would be re-injected, causing | ||
| /// argc > 2 and a test failure. | ||
| #[cfg_attr( | ||
| not(feature = "sys-thread"), | ||
| ignore = "The tokio task manager isn't available on this platform" | ||
| )] | ||
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn test_proc_exec_command_argv0() { | ||
| let wasm = run_build_script(file!(), ".").unwrap(); | ||
| let wasm_bytes = std::fs::read(&wasm).unwrap(); | ||
|
|
||
| // Create a temp dir with a wasmer.toml that has: | ||
| // - atom "inner" (the compiled wasm) | ||
| // - command "outer" using atom "inner" with extra main_args | ||
| // The command name and atom name differ on purpose so we can verify that | ||
| // argv[0] is set to the atom name (not the command name) after the fix. | ||
|
Arshia001 marked this conversation as resolved.
Outdated
|
||
| let temp = TempDir::new().unwrap(); | ||
| let wasmer_toml = r#" | ||
| [package] | ||
| name = "test/command-argv0" | ||
| version = "0.0.0" | ||
| description = "test package" | ||
|
|
||
| [[module]] | ||
| name = "inner" | ||
| source = "inner.wasm" | ||
| abi = "wasi" | ||
|
|
||
| [[command]] | ||
| name = "outer" | ||
| module = "inner" | ||
| main_args = "--extra-arg" | ||
| "#; | ||
| std::fs::write(temp.path().join("wasmer.toml"), wasmer_toml).unwrap(); | ||
| std::fs::write(temp.path().join("inner.wasm"), &wasm_bytes).unwrap(); | ||
|
|
||
| let tasks = Arc::new(TokioTaskManager::new(tokio::runtime::Handle::current())); | ||
| let mut rt = PluggableRuntime::new(Arc::clone(&tasks) as Arc<_>); | ||
| let cache = SharedCache::default().with_fallback(FileSystemCache::new( | ||
| std::env::temp_dir().join("wasmer-test-command-argv0"), | ||
| tasks.clone(), | ||
| )); | ||
|
Arshia001 marked this conversation as resolved.
|
||
| rt.set_module_cache(cache) | ||
| .set_package_loader(BuiltinPackageLoader::new()); | ||
|
|
||
| let pkg = BinaryPackage::from_dir(temp.path(), &rt).await.unwrap(); | ||
| let rt: Arc<dyn Runtime + Send + Sync> = Arc::new(rt); | ||
|
|
||
| let result = std::thread::spawn(move || { | ||
| let _guard = tasks.runtime_handle().enter(); | ||
| WasiRunner::new().run_command("outer", &pkg, RuntimeOrEngine::Runtime(Arc::clone(&rt))) | ||
| }) | ||
| .join() | ||
| .unwrap(); | ||
|
|
||
| result.unwrap(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| $CC main.c -o main |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| #include <assert.h> | ||
| #include <stdio.h> | ||
| #include <string.h> | ||
| #include <unistd.h> | ||
|
|
||
| #define CHILD_MARKER "child" | ||
| #define EXPECTED_MAIN_ARG "--extra-arg" | ||
|
|
||
| int main(int argc, char* argv[]) { | ||
| /* Child mode: invoked via execvp(argv[0], ...) from the parent. | ||
| * Verify that the command's main_args are NOT re-injected a second time. */ | ||
| for (int i = 1; i < argc; i++) { | ||
| if (strcmp(argv[i], CHILD_MARKER) == 0) { | ||
| if (argc != 2) { | ||
| fprintf(stderr, "FAIL: child expected argc=2 but got %d\n", argc); | ||
| for (int j = 0; j < argc; j++) { | ||
| fprintf(stderr, " argv[%d] = %s\n", j, argv[j]); | ||
| } | ||
| return 1; | ||
| } | ||
| /* Success: main_args were not re-injected into the re-exec'd process */ | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| /* Parent mode: verify that the command's main_args were injected for us */ | ||
| int found_main_arg = 0; | ||
| for (int i = 1; i < argc; i++) { | ||
| if (strcmp(argv[i], EXPECTED_MAIN_ARG) == 0) { | ||
| found_main_arg = 1; | ||
| break; | ||
| } | ||
| } | ||
| if (!found_main_arg) { | ||
| fprintf(stderr, "FAIL: parent expected '%s' in argv but did not find it\n", | ||
| EXPECTED_MAIN_ARG); | ||
| for (int j = 0; j < argc; j++) { | ||
| fprintf(stderr, " argv[%d] = %s\n", j, argv[j]); | ||
| } | ||
| return 1; | ||
| } | ||
|
|
||
| /* Re-exec ourselves via argv[0] with the child marker. | ||
| * After the fix, argv[0] is the atom name (not the command name), so | ||
| * execvp will find the raw atom binary without command metadata, preventing | ||
|
Arshia001 marked this conversation as resolved.
Outdated
|
||
| * main_args from being re-injected. */ | ||
| char* new_argv[] = {argv[0], CHILD_MARKER, NULL}; | ||
| execvp(argv[0], new_argv); | ||
| perror("execvp failed"); | ||
| return 1; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.