YAMAMOTO Takashi <yamam...@midokura.com> writes: > These patches, along with a few more hacks [1] I didn't include > in this patchset, allowed me to run arm64 and armv7 version of > dind image on amd64. > > [1] https://github.com/yamt/qemu/tree/linux-user-for-docker
Might be worth posting those patches next time (even if they have a RFC or !MERGE in the title for now). I had a little noodle around with testing and quickly found a few holes. It would be nice if we could have a unit test to cover these various bits as I fear it will easily break again. Feel free to use the following as a basis if you want:
>From 5d331e84f3e8763921a619647a46bc8b4c9f3207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.ben...@linaro.org> Date: Mon, 24 May 2021 10:49:55 +0100 Subject: [PATCH 1/2] tests/tcg: simple test for /proc/self behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- tests/tcg/multiarch/self.c | 114 +++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 tests/tcg/multiarch/self.c diff --git a/tests/tcg/multiarch/self.c b/tests/tcg/multiarch/self.c new file mode 100644 index 0000000000..f6ea145d16 --- /dev/null +++ b/tests/tcg/multiarch/self.c @@ -0,0 +1,114 @@ +/* + * /proc/self checks + * + * Copyright (c) 2021 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#define _GNU_SOURCE +#include <stdarg.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <errno.h> +#include <string.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> +#include <assert.h> +#include <fcntl.h> + +static void error1(const char *filename, int line, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + fprintf(stderr, "%s:%d: ", filename, line); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n"); + va_end(ap); + exit(1); +} + +static int __chk_error(const char *filename, int line, int ret) +{ + if (ret < 0) { + error1(filename, line, "%m (ret=%d, errno=%d/%s)", + ret, errno, strerror(errno)); + } + return ret; +} + +#define error(fmt, ...) error1(__FILE__, __LINE__, fmt, ## __VA_ARGS__) + +#define chk_error(ret) __chk_error(__FILE__, __LINE__, (ret)) + +#define PATH_MAX 1024 + +static void check_self_exe(struct stat *self) +{ + struct stat statbuf; + struct stat linkbuf; + chk_error(stat("/proc/self/exe", &statbuf)); + chk_error(lstat("/proc/self/exe", &linkbuf)); + assert(statbuf.st_ino != linkbuf.st_ino); + assert(statbuf.st_ino == self->st_ino); +} + +static void check_mypid(struct stat *self) +{ + pid_t me = getpid(); + char path[PATH_MAX]; + struct stat statbuf; + struct stat linkbuf; + + snprintf(&path[0], PATH_MAX, "/proc/%d/exe", me); + + chk_error(stat(path, &statbuf)); + chk_error(lstat(path, &linkbuf)); + assert(statbuf.st_ino != linkbuf.st_ino); + assert(statbuf.st_ino == self->st_ino); +} + +static void check_noncanon(struct stat *self) +{ + struct stat statbuf; + int fd_slash, fd_dot; + + fd_slash = openat(AT_FDCWD, "/proc///self/exe", O_PATH); + chk_error(fstat(fd_slash, &statbuf)); + assert(statbuf.st_ino == self->st_ino); + close(fd_slash); + + fd_dot = openat(AT_FDCWD, "/proc/./self/exe", O_PATH); + chk_error(fstat(fd_dot, &statbuf)); + assert(statbuf.st_ino == self->st_ino); + close(fd_dot); +} + +int main(int argc, char **argv) +{ + struct stat self; + + chk_error(stat(argv[0], &self)); + printf("I am %s (%d/%lu)\n", argv[0], argc, + (long unsigned int) self.st_ino); + + check_self_exe(&self); + check_mypid(&self); + check_noncanon(&self); + +#if 0 + if (argc == 2) { + printf("I think I execve'd myself\n"); + } else { + char *new_argv[3] = { argv[0], "again", NULL }; + chk_error(execve("/proc/self/exe", new_argv, NULL)); + /* should never return */ + return -1; + } +#endif + + return 0; +} base-commit: d36f3ecdc70af8941053cca8347daca757be2865 -- 2.20.1
> You can find my test setup here: > https://github.com/yamt/garbage/tree/master/binfmt-aarch64-install > > YAMAMOTO Takashi (5): > linux-user: handle /proc/self/exe for execve > linux-uesr: make exec_path realpath > linux-user: Fix the execfd case of /proc/self/exe open > linux-user: dup the execfd on start up > linux-user: Implement pivot_root > > linux-user/main.c | 14 +++++++++++++- > linux-user/qemu.h | 2 ++ > linux-user/syscall.c | 43 ++++++++++++++++++++++++++++++++++++++++--- > 3 files changed, 55 insertions(+), 4 deletions(-) I also had a go at cleaning up is_proc_self and Daniel greatly simplified it.
>From fe342309661e3fe8b9e192e6df6ef84267207dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.ben...@linaro.org> Date: Mon, 24 May 2021 12:19:18 +0100 Subject: [PATCH 2/2] linux-user: glib-ify is_proc_myself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the cost of a couple of heap allocations we can reduce the code complexity to a couple of string compares. While we are at it make the function a bool return and fixup the fake_open function prototypes. Signed-off-by: Alex Bennée <alex.ben...@linaro.org> --- v2 - use danpb's suggestion --- linux-user/syscall.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e739921e86..8af48b5f1f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7987,33 +7987,16 @@ static int open_self_auxv(void *cpu_env, int fd) return 0; } -static int is_proc_myself(const char *filename, const char *entry) -{ - if (!strncmp(filename, "/proc/", strlen("/proc/"))) { - filename += strlen("/proc/"); - if (!strncmp(filename, "self/", strlen("self/"))) { - filename += strlen("self/"); - } else if (*filename >= '1' && *filename <= '9') { - char myself[80]; - snprintf(myself, sizeof(myself), "%d/", getpid()); - if (!strncmp(filename, myself, strlen(myself))) { - filename += strlen(myself); - } else { - return 0; - } - } else { - return 0; - } - if (!strcmp(filename, entry)) { - return 1; - } - } - return 0; +static bool is_proc_myself(const char *filename, const char *entry) +{ + g_autofree char *procself = g_strdup_printf("/proc/self/%s", entry); + g_autofree char *procpid = g_strdup_printf("/proc/%d/%s", getpid(), entry); + return g_str_equal(filename, procself) || g_str_equal(filename, procpid); } #if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN) || \ defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) -static int is_proc(const char *filename, const char *entry) +static bool is_proc(const char *filename, const char *entry) { return strcmp(filename, entry) == 0; } @@ -8097,7 +8080,7 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags, struct fake_open { const char *filename; int (*fill)(void *cpu_env, int fd); - int (*cmp)(const char *s1, const char *s2); + bool (*cmp)(const char *s1, const char *s2); }; const struct fake_open *fake_open; static const struct fake_open fakes[] = { -- 2.20.1
-- Alex Bennée