Skip to content

Commit

Permalink
patches: Avoid executable path resolution
Browse files Browse the repository at this point in the history
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 <piotr.stankiewicz@docker.com>
  • Loading branch information
p1-0tr committed Feb 4, 2025
1 parent 14ebeca commit b727548
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
From 25cc3455b747b158dd7a328fd1e405a4a2c20ba8 Mon Sep 17 00:00:00 2001
From: Piotr Stankiewicz <piotr.stankiewicz@docker.com>
Date: Mon, 3 Feb 2025 16:18:28 +0100
Subject: [PATCH 8/8] Avoid executable path resolution

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@docker.com>
---
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

43 changes: 43 additions & 0 deletions test/change-workdir/chwd.go
Original file line number Diff line number Diff line change
@@ -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)
}
2 changes: 2 additions & 0 deletions test/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down
13 changes: 7 additions & 6 deletions test/test.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down Expand Up @@ -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"
}

0 comments on commit b727548

Please sign in to comment.