diff --git a/src/executor/helpers/run_with_env.rs b/src/executor/helpers/run_with_env.rs index 95c8de28..060129e7 100644 --- a/src/executor/helpers/run_with_env.rs +++ b/src/executor/helpers/run_with_env.rs @@ -36,20 +36,31 @@ pub fn wrap_with_env( mut cmd_builder: CommandBuilder, extra_env: &HashMap, ) -> Result<(CommandBuilder, NamedTempFile)> { - let env_file = create_env_file(extra_env)?; - - // Create bash command that sources the env file and runs the original command - let original_command = cmd_builder.as_command_line(); - let bash_command = format!( - "source {} && {}", - env_file.path().display(), - original_command - ); + let (bash_command, env_file) = + prefix_command_with_env(&cmd_builder.as_command_line(), extra_env)?; cmd_builder.wrap("bash", ["-c", &bash_command]); Ok((cmd_builder, env_file)) } +/// Prefixes a shell command with a `source` of the forwarded environment. +/// +/// Unlike [`wrap_with_env`], the returned value is the raw `source && ` +/// snippet without a `bash -c` wrapper, for callers that already run their argument +/// through a shell. The `source` must be the innermost layer, below any setuid or +/// file-capability binary in the chain: such binaries put the process in glibc's +/// secure-execution mode, which strips `LD_*` variables from the environment before +/// `main()`. Re-sourcing the environment after them restores those variables for the +/// benchmark. +pub fn prefix_command_with_env( + command: &str, + extra_env: &HashMap, +) -> Result<(String, NamedTempFile)> { + let env_file = create_env_file(extra_env)?; + let wrapped = format!("source {} && {}", env_file.path().display(), command); + Ok((wrapped, env_file)) +} + fn create_env_file(extra_env: &HashMap) -> Result { let system_env = get_exported_system_env()?; let base_injected_env = extra_env diff --git a/src/executor/memory/executor.rs b/src/executor/memory/executor.rs index 2b3b9c14..6040e34c 100644 --- a/src/executor/memory/executor.rs +++ b/src/executor/memory/executor.rs @@ -6,7 +6,7 @@ use crate::executor::helpers::command::CommandBuilder; use crate::executor::helpers::env::{build_path_env, get_base_injected_env}; use crate::executor::helpers::get_bench_command::get_bench_command; use crate::executor::helpers::run_command_with_log_pipe::run_command_with_log_pipe_and_callback; -use crate::executor::helpers::run_with_env::wrap_with_env; +use crate::executor::helpers::run_with_env::prefix_command_with_env; use crate::executor::helpers::run_with_sudo::is_root_user; use crate::executor::shared::fifo::RunnerFifo; use crate::executor::{ExecutionContext, Executor}; @@ -53,6 +53,14 @@ impl MemoryExecutor { // Setup memtrack IPC server let (ipc_server, server_name) = ipc::IpcOneShotServer::new()?; + // memtrack runs its command argument through `bash -c`, so the environment + // must be sourced there rather than around memtrack itself: memtrack holds + // file capabilities and runs in glibc secure-execution mode, which strips + // LD_* variables. Sourcing inside the benchmark shell restores them below + // that boundary. + let bench_command = get_bench_command(&execution_context.config)?; + let (bench_command, env_file) = prefix_command_with_env(&bench_command, &extra_env)?; + // Build the memtrack command let mut cmd_builder = CommandBuilder::new(MEMTRACK_COMMAND); cmd_builder.arg("track"); @@ -60,7 +68,7 @@ impl MemoryExecutor { cmd_builder.arg(execution_context.profile_folder.join("results")); cmd_builder.arg("--ipc-server"); cmd_builder.arg(server_name); - cmd_builder.arg(get_bench_command(&execution_context.config)?); + cmd_builder.arg(bench_command); // Set working directory if specified if let Some(cwd) = &execution_context.config.working_directory { @@ -68,8 +76,6 @@ impl MemoryExecutor { cmd_builder.current_dir(abs_cwd); } - let (cmd_builder, env_file) = wrap_with_env(cmd_builder, &extra_env)?; - Ok((ipc_server, cmd_builder, env_file)) } diff --git a/src/executor/tests.rs b/src/executor/tests.rs index 9a08399a..3b0c0e6f 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -476,4 +476,39 @@ fi }) .await; } + + // codspeed-memtrack holds file capabilities, so a non-root user executing it + // enters glibc secure-execution mode (AT_SECURE=1), which strips LD_* variables + // from the environment before main(). Forwarding the environment *around* + // memtrack instead of *into* the benchmark it spawns therefore loses + // LD_LIBRARY_PATH, and the benchmark loads the wrong shared libraries (e.g. neqo + // picking up system NSS over a locally built newer one, panicking with + // UnsupportedVersion). PATH survives this because it is not an LD_* variable. + #[test_log::test(tokio::test)] + async fn test_memory_executor_forwards_ld_library_path() { + let custom_lib = "/custom/test/lib"; + let current = std::env::var("LD_LIBRARY_PATH").unwrap_or_default(); + let modified = match current.is_empty() { + true => custom_lib.to_string(), + false => format!("{custom_lib}:{current}"), + }; + + let cmd = format!( + r#" +if ! echo "$LD_LIBRARY_PATH" | grep -q "{custom_lib}"; then + echo "FAIL: LD_LIBRARY_PATH does not contain {custom_lib}" + echo "Got LD_LIBRARY_PATH: $LD_LIBRARY_PATH" + exit 1 +fi +"# + ); + let config = memory_config(&cmd); + let (execution_context, _temp_dir) = create_test_setup(config).await; + let (_permit, _lock, mut executor) = get_memory_executor().await; + + temp_env::async_with_vars(&[("LD_LIBRARY_PATH", Some(&modified))], async { + executor.run(&execution_context, &None).await.unwrap(); + }) + .await; + } }