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

* 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

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

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