From b7275486114953594d1f84902e18627f2deee226 Mon Sep 17 00:00:00 2001 From: Piotr Stankiewicz Date: Mon, 3 Feb 2025 16:42:58 +0100 Subject: [PATCH] patches: Avoid executable path resolution The buildkit inline emulator resolves paths of the binary it executes. This is needed for readlink()/open()/... to work on /proc/self/exe correctly, in case the binary changes its working directory. However the resolution may result in unexpected args values. So avoid passing the resolved paths around, but use them when the emulated process accesses /proc/self/exe. Known issue - a shell script like `#!./chwd` will still not work as expected, as it will set /proc/self/exe to the script, rather than the actual executable. Signed-off-by: Piotr Stankiewicz --- ...008-Avoid-executable-path-resolution.patch | 93 +++++++++++++++++++ test/change-workdir/chwd.go | 43 +++++++++ test/run.sh | 2 + test/test.bats | 13 +-- 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 patches/buildkit-direct-execve-v9.2/0008-Avoid-executable-path-resolution.patch create mode 100644 test/change-workdir/chwd.go diff --git a/patches/buildkit-direct-execve-v9.2/0008-Avoid-executable-path-resolution.patch b/patches/buildkit-direct-execve-v9.2/0008-Avoid-executable-path-resolution.patch new file mode 100644 index 00000000..891a6e2f --- /dev/null +++ b/patches/buildkit-direct-execve-v9.2/0008-Avoid-executable-path-resolution.patch @@ -0,0 +1,93 @@ +From 25cc3455b747b158dd7a328fd1e405a4a2c20ba8 Mon Sep 17 00:00:00 2001 +From: Piotr Stankiewicz +Date: Mon, 3 Feb 2025 16:18:28 +0100 +Subject: [PATCH 8/8] Avoid executable path resolution + +Signed-off-by: Piotr Stankiewicz +--- + linux-user/main.c | 7 +++---- + linux-user/syscall.c | 12 ++++++------ + linux-user/user-internals.h | 1 + + 3 files changed, 10 insertions(+), 10 deletions(-) + +diff --git a/linux-user/main.c b/linux-user/main.c +index c02535d4e3..acd4c87e68 100644 +--- a/linux-user/main.c ++++ b/linux-user/main.c +@@ -68,7 +68,7 @@ + #endif + + char *exec_path; +-char real_exec_path[PATH_MAX]; ++char *real_exec_path; + + static bool opt_one_insn_per_tb; + static const char *argv0; +@@ -825,10 +825,9 @@ int main(int argc, char **argv, char **envp) + + fprintf(stderr, "DEBUG: exec_path=%s after opening execfd\n", exec_path); + ++ real_exec_path = (char *) malloc(PATH_MAX * sizeof(char)); + /* Resolve executable file name to full path name */ +- if (realpath(exec_path, real_exec_path)) { +- exec_path = real_exec_path; +- } ++ realpath(exec_path, real_exec_path); + + fprintf(stderr, "DEBUG: exec_path=%s after realpath resolution\n", exec_path); + +diff --git a/linux-user/syscall.c b/linux-user/syscall.c +index 63bdb3c154..32753c55bb 100644 +--- a/linux-user/syscall.c ++++ b/linux-user/syscall.c +@@ -8424,9 +8424,9 @@ static int maybe_do_fake_open(CPUArchState *cpu_env, int dirfd, + return -1; + } + if (safe) { +- return safe_openat(dirfd, exec_path, flags, mode); ++ return safe_openat(dirfd, real_exec_path, flags, mode); + } else { +- return openat(dirfd, exec_path, flags, mode); ++ return openat(dirfd, real_exec_path, flags, mode); + } + } + +@@ -8550,9 +8550,9 @@ ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz) + * Don't worry about sign mismatch as earlier mapping + * logic would have thrown a bad address error. + */ +- ret = MIN(strlen(exec_path), bufsiz); ++ ret = MIN(strlen(real_exec_path), bufsiz); + /* We cannot NUL terminate the string. */ +- memcpy(buf, exec_path, ret); ++ memcpy(buf, real_exec_path, ret); + } else { + ret = readlink(path(pathname), buf, bufsiz); + } +@@ -10599,9 +10599,9 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, + * Don't worry about sign mismatch as earlier mapping + * logic would have thrown a bad address error. + */ +- ret = MIN(strlen(exec_path), arg4); ++ ret = MIN(strlen(real_exec_path), arg4); + /* We cannot NUL terminate the string. */ +- memcpy(p2, exec_path, ret); ++ memcpy(p2, real_exec_path, ret); + } else { + ret = get_errno(readlinkat(arg1, path(p), p2, arg4)); + } +diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h +index 46ffc093f4..4b9876b585 100644 +--- a/linux-user/user-internals.h ++++ b/linux-user/user-internals.h +@@ -24,6 +24,7 @@ + #include "qemu/log.h" + + extern char *exec_path; ++extern char *real_exec_path; + void init_task_state(TaskState *ts); + void task_settid(TaskState *); + void stop_all_tasks(void); +-- +2.43.0 + diff --git a/test/change-workdir/chwd.go b/test/change-workdir/chwd.go new file mode 100644 index 00000000..2b8f4a34 --- /dev/null +++ b/test/change-workdir/chwd.go @@ -0,0 +1,43 @@ +package main + +import ( + "bytes" + "crypto/md5" + "fmt" + "os" +) + +func main() { + if err := os.Chdir("/tmp"); err != nil { + fmt.Println(err) + os.Exit(1) + } + if p, err := os.Readlink("/proc/self/exe"); err != nil { + fmt.Println(err) + os.Exit(1) + } else { + fmt.Println(p) + } + f, err := os.Open("/proc/self/exe") + if err != nil { + fmt.Println(err) + os.Exit(1) + } + defer f.Close() + var buf bytes.Buffer + buf.ReadFrom(f) + hash := md5.Sum(buf.Bytes()) + f2, err := os.Open(os.Args[1]) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + buf.Reset() + buf.ReadFrom(f2) + hash2 := md5.Sum(buf.Bytes()) + if hash != hash2 { + fmt.Printf("/proc/self/exe does not match %s\n", os.Args[1]) + os.Exit(1) + } + os.Exit(0) +} diff --git a/test/run.sh b/test/run.sh index ce92f9bd..c0222d1b 100755 --- a/test/run.sh +++ b/test/run.sh @@ -3,6 +3,7 @@ ln -s /usr/bin/env /work/env go build print/printargs.go go build exec/execargv0.go +go build change-workdir/chwd.go echo "testing $(uname -m)" ./test.bats @@ -18,6 +19,7 @@ fi ln -sf /crossarch/usr/bin/env /work/env GOARCH=$crossArch go build print/printargs.go GOARCH=$crossArch go build exec/execargv0.go +GOARCH=$crossArch go build change-workdir/chwd.go if ./printargs >/dev/null 2>/dev/nulll; then echo "can't test emulator because $crossEmulator emulator is installed in the kernel" diff --git a/test/test.bats b/test/test.bats index 54e28521..de097c76 100755 --- a/test/test.bats +++ b/test/test.bats @@ -83,12 +83,7 @@ execdirect() { @test "shebang-direct" { execdirect ./shebang.sh foo bar1 assert_success - if [ -n "$BINFMT_EMULATOR" ]; then - # FIXME: direct exec patches should be fixed to not prepend workdir - assert_output "./printargs /work/shebang.sh foo bar1" - else - assert_output "./printargs ./shebang.sh foo bar1" - fi + assert_output "./printargs ./shebang.sh foo bar1" } @test "relative-exec" { @@ -140,3 +135,9 @@ execdirect() { assert_success assert_output "./printargs foo bar1 bar2" } + +@test "change-workdir-and-read-self-exe" { + execdirect ./chwd /work/chwd + assert_success + assert_output "/work/chwd" +}