* [Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve @ 2019-08-07 13:54 dion 2019-08-07 13:54 ` [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve dion 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve Olivier Dion 0 siblings, 2 replies; 8+ messages in thread From: dion @ 2019-08-07 13:54 UTC (permalink / raw) To: qemu-devel; +Cc: Olivier Dion, john.ogness From: Olivier Dion <dion@linutronix.de> When the emulated process try to execve itself through /proc/self/exe, QEMU user will be executed instead of the process. The following short program demonstrated that: ---------------------------------------------------------------------- #include <stdio.h> #include <string.h> #include <unistd.h> static char *ARGV0 = "STOP"; static char *ARGV1 = "-this-is-not-an-option"; int main(int argc, char *argv[], char *environ[]) { (void)argc; if (strcmp(argv[0], ARGV0) == 0) return 0; argv[0] = ARGV0; argv[1] = ARGV1; execve("/proc/self/exe", (char **const)argv, (char **const)environ); perror("execve"); return 1; } ---------------------------------------------------------------------- Will output: ---------------------------------------------------------------------- qemu: unknown option 'this-is-not-an-option' ---------------------------------------------------------------------- Olivier Dion (1): linux-user: Handle /proc/self/exe in syscall execve linux-user/syscall.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve 2019-08-07 13:54 [Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve dion @ 2019-08-07 13:54 ` dion 2019-08-23 16:58 ` Laurent Vivier 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve Olivier Dion 1 sibling, 1 reply; 8+ messages in thread From: dion @ 2019-08-07 13:54 UTC (permalink / raw) To: qemu-devel; +Cc: Olivier Dion, john.ogness From: Olivier Dion <dion@linutronix.de> If not handled, QEMU will execve itself instead of the emulated process. This could result in potential security risk. Signed-off-by: Olivier Dion <dion@linutronix.de> --- linux-user/syscall.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 8367cb138d..1a475896a6 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7504,7 +7504,18 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, * before the execve completes and makes it the other * program's problem. */ - ret = get_errno(safe_execve(p, argp, envp)); + { + const char *pathname = p; + char real_path[PATH_MAX]; + if (is_proc_myself(pathname, "exe")) { + if (NULL == realpath(exec_path, real_path)) { + ret = get_errno(-1); + goto execve_efault; + } + pathname = real_path; + } + ret = get_errno(safe_execve(pathname, argp, envp)); + } unlock_user(p, arg1, 0); goto execve_end; -- 2.22.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve 2019-08-07 13:54 ` [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve dion @ 2019-08-23 16:58 ` Laurent Vivier 2019-09-02 17:36 ` Olivier Dion 0 siblings, 1 reply; 8+ messages in thread From: Laurent Vivier @ 2019-08-23 16:58 UTC (permalink / raw) To: dion, qemu-devel; +Cc: john.ogness Le 07/08/2019 à 15:54, dion@linutronix.de a écrit : > From: Olivier Dion <dion@linutronix.de> > > If not handled, QEMU will execve itself instead of the emulated > process. This could result in potential security risk. > Could you explain what you mean by potential security risk? Thanks, Laurent ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve 2019-08-23 16:58 ` Laurent Vivier @ 2019-09-02 17:36 ` Olivier Dion 2019-09-02 19:02 ` Laurent Vivier 0 siblings, 1 reply; 8+ messages in thread From: Olivier Dion @ 2019-09-02 17:36 UTC (permalink / raw) To: Laurent Vivier; +Cc: qemu-devel, john.ogness On 2019-08-23T12:58:43-0400, Laurent Vivier <laurent@vivier.eu> wrote: > Le 07/08/2019 à 15:54, dion@linutronix.de a écrit : > > From: Olivier Dion <dion@linutronix.de> > > > > If not handled, QEMU will execve itself instead of the emulated > > process. This could result in potential security risk. > > > Could you explain what you mean by potential security risk? I don't have any exploit in mind, but someone motivated enough could certainly find one. For example, it's possible to ask qemu static to execute another program. The main point is that an emulator should never leak informations to its environnement. If the emulated program can determine that it is being emulated, other than by an "official" way, then the emulator is at fault. -- Olivier Dion Polymtl ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve 2019-09-02 17:36 ` Olivier Dion @ 2019-09-02 19:02 ` Laurent Vivier 0 siblings, 0 replies; 8+ messages in thread From: Laurent Vivier @ 2019-09-02 19:02 UTC (permalink / raw) To: Olivier Dion; +Cc: qemu-devel, john.ogness Le 02/09/2019 à 19:36, Olivier Dion a écrit : > > On 2019-08-23T12:58:43-0400, Laurent Vivier <laurent@vivier.eu> wrote: > >> Le 07/08/2019 à 15:54, dion@linutronix.de a écrit : >>> From: Olivier Dion <dion@linutronix.de> >>> >>> If not handled, QEMU will execve itself instead of the emulated >>> process. This could result in potential security risk. >>> > >> Could you explain what you mean by potential security risk? > > I don't have any exploit in mind, but someone motivated enough could > certainly find one. For example, it's possible to ask qemu static to > execute another program. In the actual state, executing /proc/self/exe executes QEMU instead of the binary and this is a minor bug not a security risk. > The main point is that an emulator should never leak informations to its > environnement. If the emulated program can determine that it is being > emulated, other than by an "official" way, then the emulator is at > fault. It should never leak _crucial_ information (like the serial number of the host), but all emulators/hypervisors leak information (try to run lscpu/lspci in a VM). In this case, again, I don't see any security risk. Moreover qemu-user doesn't have kernel part and it has no way to elevate privilege by itself (BTW you must not run it with suid bit). We don't have a nice solution for all the files below /proc: we rely on the path name and can't check if it's in a procfs filesystem, and that is not perfect. Moreover, it doesn't work well if we use a link to access the file or a relative path. If we want a solution managing all the cases if becomes relatively complex. From my point of view, all patches are welcome. For this one: - don't introduce it as security fix but as a bug fix - propose a test case and show your fix really fixes it - you should use do_openat() with execveat() as the exec_path can be unset in the case of binfmt-misc with the credential flag (search for AT_EXECFD in QEMU code). Thanks, Laurent ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve 2019-08-07 13:54 [Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve dion 2019-08-07 13:54 ` [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve dion @ 2019-09-16 15:55 ` Olivier Dion 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 1/1] Handle /proc/self/exe in syscall execve Olivier Dion 2019-09-16 16:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve no-reply 1 sibling, 2 replies; 8+ messages in thread From: Olivier Dion @ 2019-09-16 15:55 UTC (permalink / raw) To: qemu-devel; +Cc: Olivier Dion, john.ogness * Changes from v1 - Introduce the patch as a bug fix, rather than a security fix - Use do_openat and safe_execveat instead of copying exec_path - Extensive test case example * Test case I will present a short program that demonstrated the bug, i.e. what is the expected behavior and what really happens. Then, I will explain how this patch fixes this bug. ** The program ------------------------------------------------------------------- #include <errno.h> #include <string.h> #include <unistd.h> static char *ARG0 = "STOP"; static char *ARG1 = "-this-is-not-an-option"; int main(int argc, char *argv[], char *envp[]) { (void)argc; if (0 == strcmp(argv[0], ARG0)) return 0; argv[0] = ARG0; argv[1] = ARG1; execve("/proc/self/exe", (char **const)argv, (char **const)envp); return errno; } ------------------------------------------------------------------- Note that in every cases, this program should be run with at least one argument, so that argv[1] points to something. *** Expected behavior This program when run normally, i.e. without an emulator or with this patch applied, will run two times. The first time, it will change its argv[0] and argv[1] and recursively call itself. The second time, it will stop at the string comparaison between argv[0] and the sentinel ARG0, returning 0. Thus, we expect the program to finish with error code 0 and nothing is printed to stdout&stderr. *** What really happens When emulated by qemu-user, this program will fail to call itself recursively and will instead call qemu-user. This is where ARG1 becomes useful. It's indeed set to an option that is not supported by qemu-user, and thus we expected two things 1) A message will be printed to stdout&|stderr 2) A error code different from 0 will be returned For example, I get the following output with error code 1 ------------------------------------------------------------------- qemu: unknown option 'this-is-not-an-option' ------------------------------------------------------------------- *** Automated testing The following is a quick bash script that demonstrates how to use this test case. I suppose here that qemu-user is the correct emulator for the arch of the compiled program a.out. ------------------------------------------------------------------ #!/bin/bash out=$(qemu-user ./a.out foo) ret=0 if [[ $out != "" || $? != 0 ]]; then ret=1 fi exit $ret ------------------------------------------------------------------ * Fixing the bug This patch introduces the use of safe_execveat instead of safe_execve for the emulation of execve. By using the do_openat function, we ensure that the executable file descriptor is really the one the user wants. Olivier Dion (1): Handle /proc/self/exe in syscall execve linux-user/syscall.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) -- 2.23.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/1] Handle /proc/self/exe in syscall execve 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve Olivier Dion @ 2019-09-16 15:55 ` Olivier Dion 2019-09-16 16:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve no-reply 1 sibling, 0 replies; 8+ messages in thread From: Olivier Dion @ 2019-09-16 15:55 UTC (permalink / raw) To: qemu-devel; +Cc: Olivier Dion, john.ogness If not handled, QEMU will execve itself instead of the emulated process. The function do_openat already solves the /proc/self/exe problem, so we can use it to get the executable file descriptor. We then make a safe call to execveat. Note that safe_execve has been replaced by safe_execveat, since the former is now useless. Signed-off-by: Olivier Dion <olivier.dion@polymtl.ca> --- linux-user/syscall.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e2af3c1494..68340bcb67 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -736,7 +736,7 @@ safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \ struct rusage *, rusage) safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, siginfo_t *, infop, \ int, options, struct rusage *, rusage) -safe_syscall3(int, execve, const char *, filename, char **, argv, char **, envp) +safe_syscall5(int, execveat, int, dirfd, const char *, pathname, char **, argv, char **, envp, int, flags) safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \ fd_set *, exceptfds, struct timespec *, timeout, void *, sig) safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds, @@ -7507,8 +7507,18 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, * before the execve completes and makes it the other * program's problem. */ - ret = get_errno(safe_execve(p, argp, envp)); - unlock_user(p, arg1, 0); + { + int execfd = get_errno(do_openat(cpu_env, AT_FDCWD, p, O_PATH, 0)); + unlock_user(p, arg1, 0); + if (is_error(execfd)) { + ret = execfd; + goto execve_end; + } + ret = get_errno(safe_execveat(execfd, "", + argp, envp, + AT_EMPTY_PATH)); + close(execfd); + } goto execve_end; -- 2.23.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve Olivier Dion 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 1/1] Handle /proc/self/exe in syscall execve Olivier Dion @ 2019-09-16 16:55 ` no-reply 1 sibling, 0 replies; 8+ messages in thread From: no-reply @ 2019-09-16 16:55 UTC (permalink / raw) To: olivier.dion; +Cc: olivier.dion, qemu-devel, john.ogness Patchew URL: https://patchew.org/QEMU/20190916155545.29928-1-olivier.dion@polymtl.ca/ Hi, This series failed build test on FreeBSD host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch if qemu-system-x86_64 --help >/dev/null 2>&1; then QEMU=qemu-system-x86_64 elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then QEMU=/usr/libexec/qemu-kvm else exit 1 fi make vm-build-freebsd J=21 QEMU=$QEMU exit 0 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190916155545.29928-1-olivier.dion@polymtl.ca/testing.FreeBSD/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-16 16:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-07 13:54 [Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve dion 2019-08-07 13:54 ` [Qemu-devel] [PATCH 1/1] linux-user: Handle /proc/self/exe in syscall execve dion 2019-08-23 16:58 ` Laurent Vivier 2019-09-02 17:36 ` Olivier Dion 2019-09-02 19:02 ` Laurent Vivier 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve Olivier Dion 2019-09-16 15:55 ` [Qemu-devel] [PATCH v2 1/1] Handle /proc/self/exe in syscall execve Olivier Dion 2019-09-16 16:55 ` [Qemu-devel] [PATCH v2 0/1] Handle /proc/self/exe in execve no-reply
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).