[Qemu-devel] [PATCH v5 0/6] linux-user: Reorg interp_prefix handling

[Qemu-devel] [PATCH v5 0/6] linux-user: Reorg interp_prefix handling

[Richard Henderson]

Changes since v4 (2018-01-28)
  * Protect host fds from guest manipulation.
  * Rename the interp_dirfd macro to TRY_INTERP_FD
  * Add TRY_INTERP_PATH for acct, statfs, inotify_add_watch

Changes since v3 (2017-12-29)
  * Use DO/WHILE as the control construct; wrap it in a macro.
  * Introduce linux_user_path to handle the cases *at syscalls
    do not cover.

Changes since v2 (2017-12-04)
  * Use IF as the control construct instead of SWITCH.

Changes since v1 (2016-11-??)
  * Require interp_dirfd set before trying the *at path.


r~


Richard Henderson (6):
  gdbstub: Return the fd from gdbserver_start
  linux-user: Add host_fds and manipulators
  linux-user: Check is_hostfd in do_syscall
  linux-user: Check contains_hostfd in select syscalls
  linux-user: Check is_hostfd in mmap syscalls
  linux-user: Use *at functions to implement interp_prefix

 linux-user/qemu.h    |  67 ++++++
 gdbstub.c            |   5 +-
 linux-user/elfload.c |   5 +-
 linux-user/main.c    |  53 ++++-
 linux-user/syscall.c | 485 ++++++++++++++++++++++++++++++++++++-------
 5 files changed, 532 insertions(+), 83 deletions(-)

-- 
2.17.0

[Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Richard Henderson]

This will allow us to protect gdbserver_fd from the guest.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 gdbstub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 6081e719c5..057d0d65c5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)
 int gdbserver_start(int port)
 {
     gdbserver_fd = gdbserver_open(port);
-    if (gdbserver_fd < 0)
+    if (gdbserver_fd < 0) {
         return -1;
+    }
     /* accept connections */
     if (!gdb_accept()) {
         close(gdbserver_fd);
         gdbserver_fd = -1;
         return -1;
     }
-    return 0;
+    return gdbserver_fd;
 }
 
 /* Disable gdb stub for child processes.  */
-- 
2.17.0

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Philippe Mathieu-Daudé]

Hi Richard,

On 05/31/2018 07:49 PM, Richard Henderson wrote:
> This will allow us to protect gdbserver_fd from the guest.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  gdbstub.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 6081e719c5..057d0d65c5 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)
>  int gdbserver_start(int port)
>  {
>      gdbserver_fd = gdbserver_open(port);
> -    if (gdbserver_fd < 0)
> +    if (gdbserver_fd < 0) {
>          return -1;
> +    }
>      /* accept connections */
>      if (!gdb_accept()) {
>          close(gdbserver_fd);
>          gdbserver_fd = -1;
>          return -1;
>      }
> -    return 0;
> +    return gdbserver_fd;

I agree with your change, but what about !CONFIG_USER_ONLY?

It should be safe enough documenting the different behaviors in
include/exec/gdbstub.h.

>  }
>  
>  /* Disable gdb stub for child processes.  */
> 

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Richard Henderson]

On 05/31/2018 04:15 PM, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 05/31/2018 07:49 PM, Richard Henderson wrote:
>> This will allow us to protect gdbserver_fd from the guest.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  gdbstub.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 6081e719c5..057d0d65c5 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)
>>  int gdbserver_start(int port)
>>  {
>>      gdbserver_fd = gdbserver_open(port);
>> -    if (gdbserver_fd < 0)
>> +    if (gdbserver_fd < 0) {
>>          return -1;
>> +    }
>>      /* accept connections */
>>      if (!gdb_accept()) {
>>          close(gdbserver_fd);
>>          gdbserver_fd = -1;
>>          return -1;
>>      }
>> -    return 0;
>> +    return gdbserver_fd;
> 
> I agree with your change, but what about !CONFIG_USER_ONLY?

It's still a non-negative number, so the success value is still the same.


r~

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Philippe Mathieu-Daudé]

On 05/31/2018 09:16 PM, Richard Henderson wrote:
> On 05/31/2018 04:15 PM, Philippe Mathieu-Daudé wrote:
>> Hi Richard,
>>
>> On 05/31/2018 07:49 PM, Richard Henderson wrote:
>>> This will allow us to protect gdbserver_fd from the guest.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  gdbstub.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gdbstub.c b/gdbstub.c
>>> index 6081e719c5..057d0d65c5 100644
>>> --- a/gdbstub.c
>>> +++ b/gdbstub.c
>>> @@ -1890,15 +1890,16 @@ static int gdbserver_open(int port)
>>>  int gdbserver_start(int port)
>>>  {
>>>      gdbserver_fd = gdbserver_open(port);
>>> -    if (gdbserver_fd < 0)
>>> +    if (gdbserver_fd < 0) {
>>>          return -1;
>>> +    }
>>>      /* accept connections */
>>>      if (!gdb_accept()) {
>>>          close(gdbserver_fd);
>>>          gdbserver_fd = -1;
>>>          return -1;
>>>      }
>>> -    return 0;
>>> +    return gdbserver_fd;
>>
>> I agree with your change, but what about !CONFIG_USER_ONLY?
> 
> It's still a non-negative number, so the success value is still the same.

Good point.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Peter Maydell]

On 31 May 2018 at 23:49, Richard Henderson <richard.henderson@linaro.org> wrote:
> This will allow us to protect gdbserver_fd from the guest.

Ha, I hadn't realised we already had an internal-to-QEMU filedescriptor :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Richard Henderson]

On 06/01/2018 01:59 AM, Peter Maydell wrote:
> On 31 May 2018 at 23:49, Richard Henderson <richard.henderson@linaro.org> wrote:
>> This will allow us to protect gdbserver_fd from the guest.
> 
> Ha, I hadn't realised we already had an internal-to-QEMU filedescriptor :-)

Two, in fact, if you count qemu_logfile.  ;-)


r~

Re: [Qemu-devel] [PATCH 1/6] gdbstub: Return the fd from gdbserver_start

[Laurent Vivier]

Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> This will allow us to protect gdbserver_fd from the guest.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 2/6] linux-user: Add host_fds and manipulators

[Richard Henderson]

This allows emulation of guest syscalls to reject
manipulations to fds used by the host.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h | 30 ++++++++++++++++++++++++++++++
 linux-user/main.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index c55c8e294b..33dafbe0e4 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -155,6 +155,36 @@ void task_settid(TaskState *);
 void stop_all_tasks(void);
 extern const char *qemu_uname_release;
 extern unsigned long mmap_min_addr;
+extern fd_set host_fds;
+
+/**
+ * is_hostfd:
+ * @fd: file descriptor to check
+ *
+ * Return true if @fd is being used by the host and therefore any
+ * guest system call referencing @fd should return EBADF.
+ */
+static inline bool is_hostfd(int fd)
+{
+    return fd >= 0 && fd < FD_SETSIZE && FD_ISSET(fd, &host_fds);
+}
+
+/**
+ * contains_hostfd:
+ * @fds: fd_set of descriptors to check
+ *
+ * Return true if any descriptor in @fds are being used by the host
+ * and therefore the guest system call should return EBADF.
+ */
+bool contains_hostfd(const fd_set *fds);
+
+/**
+ * add_hostfd:
+ * @fd: file descriptor to reserve
+ *
+ * Add @fd to the set of file descriptors to reserve for the host.
+ */
+void add_hostfd(int fd);
 
 /* ??? See if we can avoid exposing so much of the loader internals.  */
 
diff --git a/linux-user/main.c b/linux-user/main.c
index 78d6d3e7eb..ee3f323c08 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -49,6 +49,7 @@ static const char *cpu_type;
 unsigned long mmap_min_addr;
 unsigned long guest_base;
 int have_guest_base;
+fd_set host_fds;
 
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
@@ -112,6 +113,23 @@ int cpu_get_pic_interrupt(CPUX86State *env)
 }
 #endif
 
+bool contains_hostfd(const fd_set *fds)
+{
+    int i;
+    for (i = 0; i < ARRAY_SIZE(__FDS_BITS(fds)); ++i) {
+        if (__FDS_BITS(fds)[i] & __FDS_BITS(&host_fds)[i]) {
+            return true;
+        }
+    }
+    return true;
+}
+
+void add_hostfd(int fd)
+{
+    g_assert(fd >= 0 && fd < FD_SETSIZE);
+    FD_SET(fd, &host_fds);
+}
+
 /***********************************************************/
 /* Helper routines for implementing atomic operations.  */
 
@@ -805,12 +823,19 @@ int main(int argc, char **argv, char **envp)
 
     target_cpu_copy_regs(env, regs);
 
+    /* Prevent the guest from closing the log file.  */
+    if (qemu_logfile && qemu_logfile != stderr) {
+        add_hostfd(fileno(qemu_logfile));
+    }
+
     if (gdbstub_port) {
-        if (gdbserver_start(gdbstub_port) < 0) {
+        int fd = gdbserver_start(gdbstub_port);
+        if (fd < 0) {
             fprintf(stderr, "qemu: could not open gdbserver on port %d\n",
                     gdbstub_port);
             exit(EXIT_FAILURE);
         }
+        add_hostfd(fd);
         gdb_handlesig(cpu, 0);
     }
     cpu_loop(env);
-- 
2.17.0

Re: [Qemu-devel] [PATCH 2/6] linux-user: Add host_fds and manipulators

[Laurent Vivier]

Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> This allows emulation of guest syscalls to reject
> manipulations to fds used by the host.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/qemu.h | 30 ++++++++++++++++++++++++++++++
>  linux-user/main.c | 27 ++++++++++++++++++++++++++-
>  2 files changed, 56 insertions(+), 1 deletion(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 3/6] linux-user: Check is_hostfd in do_syscall

[Richard Henderson]

This is the vast majority of all fd inputs, but not all.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 303 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 280 insertions(+), 23 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d02c16bbc6..5339f0bc1c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8034,6 +8034,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         if (arg3 == 0)
             ret = 0;
         else {
+            if (is_hostfd(arg1)) {
+                goto ebadf;
+            }
             if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
                 goto efault;
             ret = get_errno(safe_read(arg1, p, arg3));
@@ -8045,6 +8048,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_write:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
             goto efault;
         if (fd_trans_target_to_host_data(arg1)) {
@@ -8072,6 +8078,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
     case TARGET_NR_openat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(do_openat(cpu_env, arg1, p,
@@ -8082,16 +8091,25 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #if defined(TARGET_NR_name_to_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
     case TARGET_NR_name_to_handle_at:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_name_to_handle_at(arg1, arg2, arg3, arg4, arg5);
         break;
 #endif
 #if defined(TARGET_NR_open_by_handle_at) && defined(CONFIG_OPEN_BY_HANDLE)
     case TARGET_NR_open_by_handle_at:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_open_by_handle_at(arg1, arg2, arg3);
         fd_trans_unregister(ret);
         break;
 #endif
     case TARGET_NR_close:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         fd_trans_unregister(arg1);
         ret = get_errno(close(arg1));
         break;
@@ -8155,7 +8173,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_linkat)
     case TARGET_NR_linkat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void * p2 = NULL;
             if (!arg2 || !arg4)
                 goto efault;
@@ -8180,6 +8200,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_unlinkat)
     case TARGET_NR_unlinkat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(unlinkat(arg1, p, arg3));
@@ -8311,6 +8334,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_mknodat)
     case TARGET_NR_mknodat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(mknodat(arg1, p, arg3, arg4));
@@ -8334,6 +8360,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         goto unimplemented;
 #endif
     case TARGET_NR_lseek:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(lseek(arg1, arg2, arg3));
         break;
 #if defined(TARGET_NR_getxpid) && defined(TARGET_ALPHA)
@@ -8484,7 +8513,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_futimesat)
     case TARGET_NR_futimesat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct timeval *tvp, tv[2];
             if (arg3) {
                 if (copy_from_user_timeval(&tv[0], arg3)
@@ -8520,6 +8551,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
     case TARGET_NR_faccessat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(faccessat(arg1, p, arg3, 0));
@@ -8564,7 +8598,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_renameat)
     case TARGET_NR_renameat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *p2;
             p  = lock_user_string(arg2);
             p2 = lock_user_string(arg4);
@@ -8579,7 +8615,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_renameat2)
     case TARGET_NR_renameat2:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *p2;
             p  = lock_user_string(arg2);
             p2 = lock_user_string(arg4);
@@ -8603,6 +8641,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_mkdirat)
     case TARGET_NR_mkdirat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(mkdirat(arg1, p, arg3));
@@ -8618,6 +8659,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
     case TARGET_NR_dup:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(dup(arg1));
         if (ret >= 0) {
             fd_trans_dup(arg1, ret);
@@ -8720,6 +8764,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_dup2
     case TARGET_NR_dup2:
+        if (is_hostfd(arg1) || is_hostfd(arg2)) {
+            goto ebadf;
+        }
         ret = get_errno(dup2(arg1, arg2));
         if (ret >= 0) {
             fd_trans_dup(arg1, arg2);
@@ -8731,6 +8778,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         int host_flags;
 
+        if (is_hostfd(arg1) || is_hostfd(arg2)) {
+            goto ebadf;
+        }
         if ((arg3 & ~TARGET_O_CLOEXEC) != 0) {
             return -EINVAL;
         }
@@ -9424,7 +9474,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_symlinkat)
     case TARGET_NR_symlinkat:
-        {
+        if (is_hostfd(arg2)) {
+            goto ebadf;
+        } else {
             void *p2;
             p  = lock_user_string(arg1);
             p2 = lock_user_string(arg3);
@@ -9475,7 +9527,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_readlinkat)
     case TARGET_NR_readlinkat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *p2;
             p  = lock_user_string(arg2);
             p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
@@ -9619,13 +9673,22 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg1, 0);
         break;
     case TARGET_NR_ftruncate:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(ftruncate(arg1, arg2));
         break;
     case TARGET_NR_fchmod:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fchmod(arg1, arg2));
         break;
 #if defined(TARGET_NR_fchmodat)
     case TARGET_NR_fchmodat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(fchmodat(arg1, p, arg3, 0));
@@ -9688,6 +9751,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_fstatfs:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fstatfs(arg1, &stfs));
         goto convert_statfs;
 #ifdef TARGET_NR_statfs64
@@ -9718,6 +9784,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_fstatfs64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fstatfs(arg1, &stfs));
         goto convert_statfs64;
 #endif
@@ -9732,84 +9801,135 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_accept
     case TARGET_NR_accept:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_accept4(arg1, arg2, arg3, 0);
         break;
 #endif
 #ifdef TARGET_NR_accept4
     case TARGET_NR_accept4:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_accept4(arg1, arg2, arg3, arg4);
         break;
 #endif
 #ifdef TARGET_NR_bind
     case TARGET_NR_bind:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_bind(arg1, arg2, arg3);
         break;
 #endif
 #ifdef TARGET_NR_connect
     case TARGET_NR_connect:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_connect(arg1, arg2, arg3);
         break;
 #endif
 #ifdef TARGET_NR_getpeername
     case TARGET_NR_getpeername:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_getpeername(arg1, arg2, arg3);
         break;
 #endif
 #ifdef TARGET_NR_getsockname
     case TARGET_NR_getsockname:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_getsockname(arg1, arg2, arg3);
         break;
 #endif
 #ifdef TARGET_NR_getsockopt
     case TARGET_NR_getsockopt:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_getsockopt(arg1, arg2, arg3, arg4, arg5);
         break;
 #endif
 #ifdef TARGET_NR_listen
     case TARGET_NR_listen:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(listen(arg1, arg2));
         break;
 #endif
 #ifdef TARGET_NR_recv
     case TARGET_NR_recv:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_recvfrom(arg1, arg2, arg3, arg4, 0, 0);
         break;
 #endif
 #ifdef TARGET_NR_recvfrom
     case TARGET_NR_recvfrom:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_recvfrom(arg1, arg2, arg3, arg4, arg5, arg6);
         break;
 #endif
 #ifdef TARGET_NR_recvmsg
     case TARGET_NR_recvmsg:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendrecvmsg(arg1, arg2, arg3, 0);
         break;
 #endif
 #ifdef TARGET_NR_send
     case TARGET_NR_send:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendto(arg1, arg2, arg3, arg4, 0, 0);
         break;
 #endif
 #ifdef TARGET_NR_sendmsg
     case TARGET_NR_sendmsg:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendrecvmsg(arg1, arg2, arg3, 1);
         break;
 #endif
 #ifdef TARGET_NR_sendmmsg
     case TARGET_NR_sendmmsg:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendrecvmmsg(arg1, arg2, arg3, arg4, 1);
         break;
     case TARGET_NR_recvmmsg:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendrecvmmsg(arg1, arg2, arg3, arg4, 0);
         break;
 #endif
 #ifdef TARGET_NR_sendto
     case TARGET_NR_sendto:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_sendto(arg1, arg2, arg3, arg4, arg5, arg6);
         break;
 #endif
 #ifdef TARGET_NR_shutdown
     case TARGET_NR_shutdown:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(shutdown(arg1, arg2));
         break;
 #endif
@@ -9835,6 +9955,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_setsockopt
     case TARGET_NR_setsockopt:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_setsockopt(arg1, arg2, arg3, arg4, (socklen_t) arg5);
         break;
 #endif
@@ -9938,7 +10061,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         goto do_stat;
 #endif
     case TARGET_NR_fstat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             ret = get_errno(fstat(arg1, &st));
 #if defined(TARGET_NR_stat) || defined(TARGET_NR_lstat)
         do_stat:
@@ -10110,6 +10235,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
     case TARGET_NR_fsync:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fsync(arg1));
         break;
     case TARGET_NR_clone:
@@ -10225,6 +10353,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = get_errno(getpgid(arg1));
         break;
     case TARGET_NR_fchdir:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fchdir(arg1));
         break;
 #ifdef TARGET_NR_bdflush /* not on x86_64 */
@@ -10244,7 +10375,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR__llseek /* Not on alpha */
     case TARGET_NR__llseek:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             int64_t res;
 #if !defined(__NR_llseek)
             res = lseek(arg1, ((uint64_t)arg2 << 32) | (abi_ulong)arg3, arg5);
@@ -10264,6 +10397,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_getdents
     case TARGET_NR_getdents:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #ifdef EMULATE_GETDENTS_WITH_GETDENTS
 #if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
         {
@@ -10396,7 +10532,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif /* TARGET_NR_getdents */
 #if defined(TARGET_NR_getdents64) && defined(__NR_getdents64)
     case TARGET_NR_getdents64:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct linux_dirent64 *dirp;
             abi_long count = arg3;
             if (!(dirp = lock_user(VERIFY_WRITE, arg2, count, 0)))
@@ -10454,10 +10592,19 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     goto efault;
                 }
 
+                ret = 0;
                 pfd = alloca(sizeof(struct pollfd) * nfds);
                 for (i = 0; i < nfds; i++) {
                     pfd[i].fd = tswap32(target_pfd[i].fd);
                     pfd[i].events = tswap16(target_pfd[i].events);
+                    if (is_hostfd(pfd[i].fd)) {
+                        ret = -TARGET_EBADF;
+                    }
+                }
+                if (ret < 0) {
+                    unlock_user(target_pfd, arg1,
+                                sizeof(struct target_pollfd) * nfds);
+                    goto fail;
                 }
             }
 
@@ -10541,10 +10688,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_flock:
         /* NOTE: the flock constant seems to be the same for every
            Linux platform */
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(safe_flock(arg1, arg2));
         break;
     case TARGET_NR_readv:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
             if (vec != NULL) {
                 ret = get_errno(safe_readv(arg1, vec, arg3));
@@ -10555,7 +10707,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_writev:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
             if (vec != NULL) {
                 ret = get_errno(safe_writev(arg1, vec, arg3));
@@ -10567,7 +10721,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #if defined(TARGET_NR_preadv)
     case TARGET_NR_preadv:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
             if (vec != NULL) {
                 unsigned long low, high;
@@ -10583,7 +10739,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_pwritev)
     case TARGET_NR_pwritev:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
             if (vec != NULL) {
                 unsigned long low, high;
@@ -10602,6 +10760,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #if defined(TARGET_NR_fdatasync) /* Not on alpha (osf_datasync ?) */
     case TARGET_NR_fdatasync:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fdatasync(arg1));
         break;
 #endif
@@ -10866,6 +11027,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_pread64
     case TARGET_NR_pread64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
@@ -10876,6 +11040,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (regpairs_aligned(cpu_env, num)) {
             arg4 = arg5;
             arg5 = arg6;
@@ -10971,6 +11138,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         off_t *offp = NULL;
         off_t off;
+
+        if (is_hostfd(arg1) || is_hostfd(arg2)) {
+            goto ebadf;
+        }
         if (arg3) {
             ret = get_user_sal(off, arg3);
             if (is_error(ret)) {
@@ -10992,6 +11163,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         off_t *offp = NULL;
         off_t off;
+
+        if (is_hostfd(arg1) || is_hostfd(arg2)) {
+            goto ebadf;
+        }
         if (arg3) {
             ret = get_user_s64(off, arg3);
             if (is_error(ret)) {
@@ -11059,6 +11234,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_ftruncate64
     case TARGET_NR_ftruncate64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 	ret = target_ftruncate64(cpu_env, arg1, arg2, arg3, arg4);
 	break;
 #endif
@@ -11084,6 +11262,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_fstat64
     case TARGET_NR_fstat64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fstat(arg1, &st));
         if (!is_error(ret))
             ret = host_to_target_stat64(cpu_env, arg2, &st);
@@ -11096,6 +11277,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_newfstatat
     case TARGET_NR_newfstatat:
 #endif
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2)))
             goto efault;
         ret = get_errno(fstatat(arg1, path(p), &st, arg4));
@@ -11184,6 +11368,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #if defined(TARGET_NR_fchownat)
     case TARGET_NR_fchownat:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (!(p = lock_user_string(arg2))) 
             goto efault;
         ret = get_errno(fchownat(arg1, p, low2highuid(arg3),
@@ -11525,6 +11712,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_fchown32
     case TARGET_NR_fchown32:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(fchown(arg1, arg2, arg3));
         break;
 #endif
@@ -11626,6 +11816,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
          * Note that offset and len are both 64-bit so appear as
          * pairs of 32-bit registers.
          */
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = posix_fadvise(arg1, target_offset64(arg3, arg4),
                             target_offset64(arg5, arg6), arg2);
         ret = -host_to_target_errno(ret);
@@ -11636,6 +11829,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #ifdef TARGET_NR_fadvise64_64
     case TARGET_NR_fadvise64_64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #if defined(TARGET_PPC) || defined(TARGET_XTENSA)
         /* 6 args: fd, advice, offset (high, low), len (high, low) */
         ret = arg2;
@@ -11664,6 +11860,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #ifdef TARGET_NR_fadvise64
     case TARGET_NR_fadvise64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         /* 5 args: fd, offset (high, low), len, advice */
         if (regpairs_aligned(cpu_env, num)) {
             /* offset is in (3,4), len in 5 and advice in 6 */
@@ -11686,6 +11885,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_fadvise64
     case TARGET_NR_fadvise64:
 #endif
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #ifdef TARGET_S390X
         switch (arg4) {
         case 4: arg4 = POSIX_FADV_NOREUSE + 1; break; /* make sure it's an invalid value */
@@ -11711,6 +11913,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if TARGET_ABI_BITS == 32
     case TARGET_NR_fcntl64:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
     {
 	int cmd;
 	struct flock64 fl;
@@ -11858,7 +12063,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_fsetxattr:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *n, *v = 0;
             if (arg3) {
                 v = lock_user(VERIFY_READ, arg3, arg4, 1);
@@ -11905,7 +12112,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_fgetxattr:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *n, *v = 0;
             if (arg3) {
                 v = lock_user(VERIFY_WRITE, arg3, arg4, 0);
@@ -11944,7 +12153,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
     case TARGET_NR_fremovexattr:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             void *n;
             n = lock_user_string(arg2);
             if (n) {
@@ -12095,7 +12306,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #if defined(TARGET_NR_utimensat)
     case TARGET_NR_utimensat:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct timespec *tsp, ts[2];
             if (!arg3) {
                 tsp = NULL;
@@ -12141,6 +12354,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
     case TARGET_NR_inotify_add_watch:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         p = lock_user_string(arg2);
         ret = get_errno(sys_inotify_add_watch(arg1, path(p), arg3));
         unlock_user(p, arg2, 0);
@@ -12148,6 +12364,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
     case TARGET_NR_inotify_rm_watch:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(sys_inotify_rm_watch(arg1, arg2));
         break;
 #endif
@@ -12248,14 +12467,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef CONFIG_SPLICE
 #ifdef TARGET_NR_tee
     case TARGET_NR_tee:
-        {
+        if (is_hostfd(arg1) || is_hostfd(arg2)) {
+            goto ebadf;
+        } else {
             ret = get_errno(tee(arg1,arg2,arg3,arg4));
         }
         break;
 #endif
 #ifdef TARGET_NR_splice
     case TARGET_NR_splice:
-        {
+        if (is_hostfd(arg1) || is_hostfd(arg3)) {
+            goto ebadf;
+        } else {
             loff_t loff_in, loff_out;
             loff_t *ploff_in = NULL, *ploff_out = NULL;
             if (arg2) {
@@ -12285,8 +12508,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #endif
 #ifdef TARGET_NR_vmsplice
-	case TARGET_NR_vmsplice:
-        {
+    case TARGET_NR_vmsplice:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
             if (vec != NULL) {
                 ret = get_errno(vmsplice(arg1, vec, arg3, arg4));
@@ -12327,6 +12552,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif /* CONFIG_EVENTFD  */
 #if defined(CONFIG_FALLOCATE) && defined(TARGET_NR_fallocate)
     case TARGET_NR_fallocate:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #if TARGET_ABI_BITS == 32
         ret = get_errno(fallocate(arg1, arg2, target_offset64(arg3, arg4),
                                   target_offset64(arg5, arg6)));
@@ -12338,6 +12566,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #if defined(CONFIG_SYNC_FILE_RANGE)
 #if defined(TARGET_NR_sync_file_range)
     case TARGET_NR_sync_file_range:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #if TARGET_ABI_BITS == 32
 #if defined(TARGET_MIPS)
         ret = get_errno(sync_file_range(arg1, target_offset64(arg3, arg4),
@@ -12354,6 +12585,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #if defined(TARGET_NR_sync_file_range2)
     case TARGET_NR_sync_file_range2:
         /* This is like sync_file_range but the arguments are reordered */
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
 #if TARGET_ABI_BITS == 32
         ret = get_errno(sync_file_range(arg1, target_offset64(arg3, arg4),
                                         target_offset64(arg5, arg6), arg2));
@@ -12365,11 +12599,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #if defined(TARGET_NR_signalfd4)
     case TARGET_NR_signalfd4:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_signalfd4(arg1, arg2, arg4);
         break;
 #endif
 #if defined(TARGET_NR_signalfd)
     case TARGET_NR_signalfd:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = do_signalfd4(arg1, arg2, 0);
         break;
 #endif
@@ -12389,6 +12629,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         struct epoll_event ep;
         struct epoll_event *epp = 0;
+
+        if (is_hostfd(arg1) || is_hostfd(arg3)) {
+            goto ebadf;
+        }
         if (arg4) {
             struct target_epoll_event *target_ep;
             if (!lock_user_struct(VERIFY_READ, target_ep, arg4, 1)) {
@@ -12422,6 +12666,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         int maxevents = arg3;
         int timeout = arg4;
 
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         if (maxevents <= 0 || maxevents > TARGET_EP_MAX_EVENTS) {
             ret = -TARGET_EINVAL;
             break;
@@ -12698,7 +12945,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #if defined(TARGET_NR_timerfd_gettime) && defined(CONFIG_TIMERFD)
     case TARGET_NR_timerfd_gettime:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct itimerspec its_curr;
 
             ret = get_errno(timerfd_gettime(arg1, &its_curr));
@@ -12712,7 +12961,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #if defined(TARGET_NR_timerfd_settime) && defined(CONFIG_TIMERFD)
     case TARGET_NR_timerfd_settime:
-        {
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        } else {
             struct itimerspec its_new, its_old, *p_new;
 
             if (arg3) {
@@ -12747,6 +12998,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 #if defined(TARGET_NR_setns) && defined(CONFIG_SETNS)
     case TARGET_NR_setns:
+        if (is_hostfd(arg1)) {
+            goto ebadf;
+        }
         ret = get_errno(setns(arg1, arg2));
         break;
 #endif
@@ -12781,4 +13035,7 @@ fail:
 efault:
     ret = -TARGET_EFAULT;
     goto fail;
+ebadf:
+    ret = -TARGET_EBADF;
+    goto fail;
 }
-- 
2.17.0

Re: [Qemu-devel] [PATCH 3/6] linux-user: Check is_hostfd in do_syscall

[Laurent Vivier]

Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> This is the vast majority of all fd inputs, but not all.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall.c | 303 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 280 insertions(+), 23 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 4/6] linux-user: Check contains_hostfd in select syscalls

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5339f0bc1c..b98125829b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1506,6 +1506,11 @@ static abi_long do_select(int n,
     if (ret) {
         return ret;
     }
+    if (contains_hostfd(&rfds) ||
+        contains_hostfd(&wfds) ||
+        contains_hostfd(&efds)) {
+        return -TARGET_EBADF;
+    }
 
     if (target_tv_addr) {
         if (copy_from_user_timeval(&tv, target_tv_addr))
@@ -9392,6 +9397,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (ret) {
                 goto fail;
             }
+            if (contains_hostfd(&rfds) ||
+                contains_hostfd(&wfds) ||
+                contains_hostfd(&efds)) {
+                goto ebadf;
+            }
 
             /*
              * This takes a timespec, and not a timeval, so we cannot
-- 
2.17.0

Re: [Qemu-devel] [PATCH 4/6] linux-user: Check contains_hostfd in select syscalls

[Laurent Vivier]

Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 5/6] linux-user: Check is_hostfd in mmap syscalls

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b98125829b..d7513d5dac 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9605,11 +9605,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             v5 = tswapal(v[4]);
             v6 = tswapal(v[5]);
             unlock_user(v, arg1, 0);
+            if (is_hostfd(v5)) {
+                goto ebadf;
+            }
             ret = get_errno(target_mmap(v1, v2, v3,
                                         target_to_host_bitmask(v4, mmap_flags_tbl),
                                         v5, v6));
         }
 #else
+        if (is_hostfd(arg5)) {
+            goto ebadf;
+        }
         ret = get_errno(target_mmap(arg1, arg2, arg3,
                                     target_to_host_bitmask(arg4, mmap_flags_tbl),
                                     arg5,
@@ -9622,6 +9628,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifndef MMAP_SHIFT
 #define MMAP_SHIFT 12
 #endif
+        if (is_hostfd(arg5)) {
+            goto ebadf;
+        }
         ret = get_errno(target_mmap(arg1, arg2, arg3,
                                     target_to_host_bitmask(arg4, mmap_flags_tbl),
                                     arg5,
-- 
2.17.0

Re: [Qemu-devel] [PATCH 5/6] linux-user: Check is_hostfd in mmap syscalls

[Laurent Vivier]

Le 01/06/2018 à 00:49, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

[Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Richard Henderson]

If the interp_prefix is a complete chroot, it may have a *lot* of files.
Setting up the cache for this is quite expensive.

For the most part, we can use the *at versions of various syscalls to
attempt the operation in the prefix.  For the few cases that remain,
attempt the operation in the prefix via concatenation and then retry
if that fails.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h    |  37 ++++++++++
 linux-user/elfload.c |   5 +-
 linux-user/main.c    |  26 ++++++-
 linux-user/syscall.c | 163 +++++++++++++++++++++++++++++--------------
 4 files changed, 174 insertions(+), 57 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 33dafbe0e4..05a82a3628 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -471,7 +471,44 @@ void mmap_fork_start(void);
 void mmap_fork_end(int child);
 
 /* main.c */
+extern int interp_dirfd;
 extern unsigned long guest_stack_size;
+char *interp_prefix_path(const char *path);
+
+/* If PATH is absolute, attempt an operation first within interp_dirfd,
+ * using OPENAT_EXPR.  If that fails with ENOENT, or if PATH is not
+ * absolute, only use NORMAL_EXPR.
+ */
+#define TRY_INTERP_FD(RET, PATH, OPENAT_EXPR, NORMAL_EXPR)  \
+    do {                                                    \
+        if (interp_dirfd >= 0 && (PATH)[0] == '/') {        \
+            RET = OPENAT_EXPR;                              \
+            if (RET != -1 || errno != ENOENT) {             \
+                break;                                      \
+            }                                               \
+        }                                                   \
+        RET = NORMAL_EXPR;                                  \
+    } while (0)
+
+/* If PATH is absolute, attempt an operation first with interp_prefix
+ * prefixed.  If that fails with ENOENT, or if PATH is not absolute,
+ * only attempt with PATH.
+ */
+#define TRY_INTERP_PATH(RET, PATH, EXPR)                    \
+    do {                                                    \
+        char *new_##PATH = interp_prefix_path(PATH);        \
+        if (new_##PATH) {                                   \
+            __typeof(PATH) save_##PATH = PATH;              \
+            PATH = new_##PATH;                              \
+            RET = EXPR;                                     \
+            free(new_##PATH);                               \
+            PATH = save_##PATH;                             \
+            if (RET != -1 || errno != ENOENT) {             \
+                break;                                      \
+            }                                               \
+        }                                                   \
+        RET = EXPR;                                         \
+    } while (0)
 
 /* user access */
 
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 13bc78d0c8..abdf5bbf01 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -6,7 +6,6 @@
 
 #include "qemu.h"
 #include "disas/disas.h"
-#include "qemu/path.h"
 
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
@@ -2375,7 +2374,9 @@ static void load_elf_interp(const char *filename, struct image_info *info,
 {
     int fd, retval;
 
-    fd = open(path(filename), O_RDONLY);
+    TRY_INTERP_FD(fd, filename,
+                  openat(interp_dirfd, filename + 1, O_RDONLY),
+                  open(filename, O_RDONLY));
     if (fd < 0) {
         goto exit_perror;
     }
diff --git a/linux-user/main.c b/linux-user/main.c
index ee3f323c08..4e956fc00c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -23,7 +23,6 @@
 
 #include "qapi/error.h"
 #include "qemu.h"
-#include "qemu/path.h"
 #include "qemu/config-file.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
@@ -89,9 +88,27 @@ unsigned long reserved_va;
 
 static void usage(int exitcode);
 
+int interp_dirfd;
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release;
 
+char *interp_prefix_path(const char *path)
+{
+    size_t i_len, p_len;
+    char *ret;
+
+    if (interp_prefix == NULL || path[0] != '/') {
+        return NULL;
+    }
+    i_len = strlen(interp_prefix);
+    p_len = strlen(path) + 1;
+    ret = g_malloc(i_len + p_len);
+
+    memcpy(ret, interp_prefix, i_len);
+    memcpy(ret + i_len, path, p_len);
+    return ret;
+}
+
 /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
    we allocate a bigger stack. Need a better solution, for example
    by remapping the process stack directly at the right place */
@@ -671,7 +688,12 @@ int main(int argc, char **argv, char **envp)
     memset(&bprm, 0, sizeof (bprm));
 
     /* Scan interp_prefix dir for replacement files. */
-    init_paths(interp_prefix);
+    interp_dirfd = open(interp_prefix, O_CLOEXEC | O_DIRECTORY | O_PATH);
+    if (interp_dirfd >= 0) {
+        add_hostfd(interp_dirfd);
+    } else {
+        interp_prefix = NULL;
+    }
 
     init_qemu_uname_release();
 
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d7513d5dac..b75dd9a5bc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -19,7 +19,6 @@
 #define _ATFILE_SOURCE
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
-#include "qemu/path.h"
 #include <elf.h>
 #include <endian.h>
 #include <grp.h>
@@ -7401,7 +7400,10 @@ static abi_long do_name_to_handle_at(abi_long dirfd, abi_long pathname,
     fh = g_malloc0(total_size);
     fh->handle_bytes = size;
 
-    ret = get_errno(name_to_handle_at(dirfd, path(name), fh, &mid, flags));
+    TRY_INTERP_FD(ret, name,
+                  name_to_handle_at(interp_dirfd, name + 1, fh, &mid, flags),
+                  name_to_handle_at(dirfd, name, fh, &mid, flags));
+    ret = get_errno(ret);
     unlock_user(name, pathname, 0);
 
     /* man name_to_handle_at(2):
@@ -7777,6 +7779,7 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags,
 #endif
         { NULL, NULL, NULL }
     };
+    int ret;
 
     if (is_proc_myself(pathname, "exe")) {
         int execfd = qemu_getauxval(AT_EXECFD);
@@ -7816,7 +7819,10 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags,
         return fd;
     }
 
-    return safe_openat(dirfd, path(pathname), flags, mode);
+    TRY_INTERP_FD(ret, pathname,
+                  safe_openat(interp_dirfd, pathname + 1, flags, mode),
+                  safe_openat(dirfd, pathname, flags, mode));
+    return ret;
 }
 
 #define TIMER_MAGIC 0x0caf0000
@@ -7969,6 +7975,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     struct stat st;
     struct statfs stfs;
     void *p;
+    char *fn;
 
 #if defined(DEBUG_ERESTARTSYS)
     /* Debug-only code for exercising the syscall-restart code paths
@@ -8531,10 +8538,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             } else {
                 tvp = NULL;
             }
-            if (!(p = lock_user_string(arg2)))
+            if (!(fn = lock_user_string(arg2))) {
                 goto efault;
-            ret = get_errno(futimesat(arg1, path(p), tvp));
-            unlock_user(p, arg2, 0);
+            }
+            TRY_INTERP_FD(ret, fn,
+                          futimesat(interp_dirfd, fn + 1, tvp),
+                          futimesat(arg1, fn, tvp));
+            ret = get_errno(ret);
+            unlock_user(fn, arg2, 0);
         }
         break;
 #endif
@@ -8548,10 +8559,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_access
     case TARGET_NR_access:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(access(path(p), arg2));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      faccessat(interp_dirfd, fn + 1, arg2, 0),
+                      access(fn, arg2));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
         break;
 #endif
 #if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
@@ -8559,10 +8574,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         if (is_hostfd(arg1)) {
             goto ebadf;
         }
-        if (!(p = lock_user_string(arg2)))
+        if (!(fn = lock_user_string(arg2))) {
             goto efault;
-        ret = get_errno(faccessat(arg1, p, arg3, 0));
-        unlock_user(p, arg2, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      faccessat(interp_dirfd, fn + 1, arg3, 0),
+                      faccessat(arg1, fn, arg3, 0));
+        ret = get_errno(ret);
+        unlock_user(fn, arg2, 0);
         break;
 #endif
 #ifdef TARGET_NR_nice /* not on alpha */
@@ -8713,10 +8732,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         if (arg1 == 0) {
             ret = get_errno(acct(NULL));
         } else {
-            if (!(p = lock_user_string(arg1)))
+            if (!(fn = lock_user_string(arg1))) {
                 goto efault;
-            ret = get_errno(acct(path(p)));
-            unlock_user(p, arg1, 0);
+            }
+            TRY_INTERP_PATH(ret, fn, acct(fn));
+            ret = get_errno(ret);
+            unlock_user(fn, arg1, 0);
         }
         break;
 #ifdef TARGET_NR_umount2
@@ -9507,14 +9528,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_readlink:
         {
             void *p2;
-            p = lock_user_string(arg1);
+            fn = lock_user_string(arg1);
             p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0);
-            if (!p || !p2) {
+            if (!fn || !p2) {
                 ret = -TARGET_EFAULT;
             } else if (!arg3) {
                 /* Short circuit this for the magic exe check. */
                 ret = -TARGET_EINVAL;
-            } else if (is_proc_myself((const char *)p, "exe")) {
+            } else if (is_proc_myself(fn, "exe")) {
                 char real[PATH_MAX], *temp;
                 temp = realpath(exec_path, real);
                 /* Return value is # of bytes that we wrote to the buffer. */
@@ -9528,10 +9549,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     memcpy(p2, real, ret);
                 }
             } else {
-                ret = get_errno(readlink(path(p), p2, arg3));
+                TRY_INTERP_FD(ret, fn,
+                              readlinkat(interp_dirfd, fn + 1, p2, arg3),
+                              readlink(fn, p2, arg3));
+                ret = get_errno(ret);
             }
             unlock_user(p2, arg2, ret);
-            unlock_user(p, arg1, 0);
+            unlock_user(fn, arg1, 0);
         }
         break;
 #endif
@@ -9541,20 +9565,23 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             goto ebadf;
         } else {
             void *p2;
-            p  = lock_user_string(arg2);
+            fn = lock_user_string(arg2);
             p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
-            if (!p || !p2) {
+            if (!fn || !p2) {
                 ret = -TARGET_EFAULT;
-            } else if (is_proc_myself((const char *)p, "exe")) {
+            } else if (is_proc_myself(fn, "exe")) {
                 char real[PATH_MAX], *temp;
                 temp = realpath(exec_path, real);
                 ret = temp == NULL ? get_errno(-1) : strlen(real) ;
                 snprintf((char *)p2, arg4, "%s", real);
             } else {
-                ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
+                TRY_INTERP_FD(ret, fn,
+                              readlinkat(interp_dirfd, fn + 1, p2, arg4),
+                              readlinkat(arg1, fn, p2, arg4));
+                ret = get_errno(ret);
             }
             unlock_user(p2, arg3, ret);
-            unlock_user(p, arg2, 0);
+            unlock_user(fn, arg2, 0);
         }
         break;
 #endif
@@ -9739,10 +9766,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         goto unimplemented;
 #endif
     case TARGET_NR_statfs:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(statfs(path(p), &stfs));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_PATH(ret, fn, statfs(fn, &stfs));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
     convert_statfs:
         if (!is_error(ret)) {
             struct target_statfs *target_stfs;
@@ -9777,10 +9806,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         goto convert_statfs;
 #ifdef TARGET_NR_statfs64
     case TARGET_NR_statfs64:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(statfs(path(p), &stfs));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_PATH(ret, fn, statfs(fn, &stfs));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
     convert_statfs64:
         if (!is_error(ret)) {
             struct target_statfs64 *target_stfs;
@@ -10065,18 +10096,26 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         break;
 #ifdef TARGET_NR_stat
     case TARGET_NR_stat:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(stat(path(p), &st));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      fstatat(interp_dirfd, fn + 1, &st, 0),
+                      stat(fn, &st));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
         goto do_stat;
 #endif
 #ifdef TARGET_NR_lstat
     case TARGET_NR_lstat:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(lstat(path(p), &st));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      fstatat(interp_dirfd, fn + 1, &st, AT_SYMLINK_NOFOLLOW),
+                      lstat(fn, &st));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
         goto do_stat;
 #endif
     case TARGET_NR_fstat:
@@ -11261,20 +11300,28 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_stat64
     case TARGET_NR_stat64:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(stat(path(p), &st));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      fstatat(interp_dirfd, fn + 1, &st, 0),
+                      stat(fn, &st));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
         if (!is_error(ret))
             ret = host_to_target_stat64(cpu_env, arg2, &st);
         break;
 #endif
 #ifdef TARGET_NR_lstat64
     case TARGET_NR_lstat64:
-        if (!(p = lock_user_string(arg1)))
+        if (!(fn = lock_user_string(arg1))) {
             goto efault;
-        ret = get_errno(lstat(path(p), &st));
-        unlock_user(p, arg1, 0);
+        }
+        TRY_INTERP_FD(ret, fn,
+                      fstatat(interp_dirfd, fn + 1, &st, AT_SYMLINK_NOFOLLOW),
+                      lstat(fn, &st));
+        ret = get_errno(ret);
+        unlock_user(fn, arg1, 0);
         if (!is_error(ret))
             ret = host_to_target_stat64(cpu_env, arg2, &st);
         break;
@@ -11299,9 +11346,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         if (is_hostfd(arg1)) {
             goto ebadf;
         }
-        if (!(p = lock_user_string(arg2)))
+        if (!(fn = lock_user_string(arg2))) {
             goto efault;
-        ret = get_errno(fstatat(arg1, path(p), &st, arg4));
+        }
+        TRY_INTERP_FD(ret, fn,
+                      fstatat(interp_dirfd, fn + 1, &st, arg4),
+                      fstatat(arg1, fn, &st, arg4));
+        ret = get_errno(ret);
+        unlock_user(fn, arg2, 0);
         if (!is_error(ret))
             ret = host_to_target_stat64(cpu_env, arg3, &st);
         break;
@@ -12339,12 +12391,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!arg2)
                 ret = get_errno(sys_utimensat(arg1, NULL, tsp, arg4));
             else {
-                if (!(p = lock_user_string(arg2))) {
-                    ret = -TARGET_EFAULT;
-                    goto fail;
+                if (!(fn = lock_user_string(arg2))) {
+                    goto efault;
                 }
-                ret = get_errno(sys_utimensat(arg1, path(p), tsp, arg4));
-                unlock_user(p, arg2, 0);
+                TRY_INTERP_FD(ret, fn,
+                              sys_utimensat(interp_dirfd, fn + 1, tsp, arg4),
+                              sys_utimensat(arg1, fn, tsp, arg4));
+                ret = get_errno(ret);
+                unlock_user(fn, arg2, 0);
             }
         }
 	break;
@@ -12376,9 +12430,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         if (is_hostfd(arg1)) {
             goto ebadf;
         }
-        p = lock_user_string(arg2);
-        ret = get_errno(sys_inotify_add_watch(arg1, path(p), arg3));
-        unlock_user(p, arg2, 0);
+        if (!(fn = lock_user_string(arg2))) {
+            goto efault;
+        }
+        TRY_INTERP_PATH(ret, fn, sys_inotify_add_watch(arg1, fn, arg3));
+        ret = get_errno(ret);
+        unlock_user(fn, arg2, 0);
         break;
 #endif
 #if defined(TARGET_NR_inotify_rm_watch) && defined(__NR_inotify_rm_watch)
-- 
2.17.0

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

On 01/06/2018 00:49, Richard Henderson wrote:
> If the interp_prefix is a complete chroot, it may have a *lot* of files.
> Setting up the cache for this is quite expensive.
> 
> For the most part, we can use the *at versions of various syscalls to
> attempt the operation in the prefix.  For the few cases that remain,
> attempt the operation in the prefix via concatenation and then retry
> if that fails.
> 

I like the idea, but it breaks real chroot.

You can test it with:

wget
https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh

then

sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch

It fails with:

Reading package lists... Done
E: Method /usr/lib/apt/methods/http did not start correctly
E: Failed to fetch http://ftp.us.debian.org/debian/dists/stretch/InRelease
E: Some index files failed to download. They have been ignored, or old
ones used instead.
stretch s390x FAILED


Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Richard Henderson]

On 06/03/2018 06:04 PM, Laurent Vivier wrote:
> On 01/06/2018 00:49, Richard Henderson wrote:
>> If the interp_prefix is a complete chroot, it may have a *lot* of files.
>> Setting up the cache for this is quite expensive.
>>
>> For the most part, we can use the *at versions of various syscalls to
>> attempt the operation in the prefix.  For the few cases that remain,
>> attempt the operation in the prefix via concatenation and then retry
>> if that fails.
>>
> 
> I like the idea, but it breaks real chroot.
> 
> You can test it with:
> 
> wget
> https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh
> 
> then
> 
> sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch

*shrug* Works for me, at least as far as I can test.

Your script doesn't work outside debian, lacking debootstrap.
At the moment, I can't build on debian *at all*.  Some bit of the build
infrastructure is off and QEMU_FULL_VERSION gets incorrectly defined.  I have
no idea how or why it is different than Fedora.

On Fedora 28, one can no longer build a static qemu.  We depend on libraries
for which Fedora no longer ships static versions.

Do you have some specific binary that fails?


r~

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

On 05/06/2018 07:27, Richard Henderson wrote:
> On 06/03/2018 06:04 PM, Laurent Vivier wrote:
>> On 01/06/2018 00:49, Richard Henderson wrote:
>>> If the interp_prefix is a complete chroot, it may have a *lot* of files.
>>> Setting up the cache for this is quite expensive.
>>>
>>> For the most part, we can use the *at versions of various syscalls to
>>> attempt the operation in the prefix.  For the few cases that remain,
>>> attempt the operation in the prefix via concatenation and then retry
>>> if that fails.
>>>
>>
>> I like the idea, but it breaks real chroot.
>>
>> You can test it with:
>>
>> wget
>> https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh
>>
>> then
>>
>> sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch
> 
> *shrug* Works for me, at least as far as I can test.
> 
> Your script doesn't work outside debian, lacking debootstrap
Yes, but Fedora has debootstrap.

> At the moment, I can't build on debian *at all*.  Some bit of the build
> infrastructure is off and QEMU_FULL_VERSION gets incorrectly defined.  I have
> no idea how or why it is different than Fedora.
> 
> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
> for which Fedora no longer ships static versions.

I build my test binaries on Fedora 28.

My configure is:

./configure' '--enable-linux-user' '--disable-system' '--static'
'--disable-tools' '--disable-docs'

> Do you have some specific binary that fails?

for qemu, qemu-ppc64le fails, and I think it"s "apt-get update" that is
executed.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Richard Henderson]

On 06/04/2018 11:33 PM, Laurent Vivier wrote:
> Yes, but Fedora has debootstrap.

Fair enough.  That's new to me.

I don't see how your script is supposed to work wrt binfmt-misc.  If you're
copying in the static qemu, you need to adjust, or install, the interpreter.

Therefore I am unsurprised to find that your script fails at

    cp "$QEMU_PATH" $CHROOT/ && \
    chroot $CHROOT ./debootstrap/debootstrap --second-stage || exit


r~

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

Le 05/06/2018 à 16:18, Richard Henderson a écrit :
> On 06/04/2018 11:33 PM, Laurent Vivier wrote:
>> Yes, but Fedora has debootstrap.
> 
> Fair enough.  That's new to me.
> 
> I don't see how your script is supposed to work wrt binfmt-misc.  If you're
> copying in the static qemu, you need to adjust, or install, the interpreter.

Yes, I use scripts/qemu-binfmt-conf.sh to do that:

./scripts/qemu-binfmt-conf.sh --qemu-path / --systemd ppc64le
--credential yes
systemctl restart systemd-binfmt.service

And it is restored on any reboot.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Peter Maydell]

On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
> for which Fedora no longer ships static versions.

Do you mean you can't build the system binaries statically (that's
never really been very supported), or that you can't do a build
with configure --disable-system --disable-tools --static ?

The latter would be more concerning.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

Le 05/06/2018 à 12:52, Peter Maydell a écrit :
> On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
>> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
>> for which Fedora no longer ships static versions.
> 
> Do you mean you can't build the system binaries statically (that's
> never really been very supported), or that you can't do a build
> with configure --disable-system --disable-tools --static ?
> 
> The latter would be more concerning.

On Fedora I have also to disable gcrypt, nettle and libiscsi because we
don't have the statically linked libraries, but we don't need them with
linux-user.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Peter Maydell]

On 5 June 2018 at 13:05, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 05/06/2018 à 12:52, Peter Maydell a écrit :
>> On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
>>> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
>>> for which Fedora no longer ships static versions.
>>
>> Do you mean you can't build the system binaries statically (that's
>> never really been very supported), or that you can't do a build
>> with configure --disable-system --disable-tools --static ?
>>
>> The latter would be more concerning.
>
> On Fedora I have also to disable gcrypt, nettle and libiscsi because we
> don't have the statically linked libraries, but we don't need them with
> linux-user.

Do we actually try to link with those for linux-user? If not,
maybe we could make configure a bit more helpful about not
requiring you to manually --disable-foo them.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

Le 05/06/2018 à 14:14, Peter Maydell a écrit :
> On 5 June 2018 at 13:05, Laurent Vivier <laurent@vivier.eu> wrote:
>> Le 05/06/2018 à 12:52, Peter Maydell a écrit :
>>> On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
>>>> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
>>>> for which Fedora no longer ships static versions.
>>>
>>> Do you mean you can't build the system binaries statically (that's
>>> never really been very supported), or that you can't do a build
>>> with configure --disable-system --disable-tools --static ?
>>>
>>> The latter would be more concerning.
>>
>> On Fedora I have also to disable gcrypt, nettle and libiscsi because we
>> don't have the statically linked libraries, but we don't need them with
>> linux-user.
> 
> Do we actually try to link with those for linux-user? If not,
> maybe we could make configure a bit more helpful about not
> requiring you to manually --disable-foo them.

They are used by tests/qemu-iotests/socket_scm_helper and
qemu-bridge-helper.

Perhaps we can disable qemu-iotests with linux-user only and to not
build qemu-bridge-helper when tools are disabled?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Daniel P. Berrangé]

On Tue, Jun 05, 2018 at 02:23:56PM +0200, Laurent Vivier wrote:
> Le 05/06/2018 à 14:14, Peter Maydell a écrit :
> > On 5 June 2018 at 13:05, Laurent Vivier <laurent@vivier.eu> wrote:
> >> Le 05/06/2018 à 12:52, Peter Maydell a écrit :
> >>> On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
> >>>> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
> >>>> for which Fedora no longer ships static versions.
> >>>
> >>> Do you mean you can't build the system binaries statically (that's
> >>> never really been very supported), or that you can't do a build
> >>> with configure --disable-system --disable-tools --static ?
> >>>
> >>> The latter would be more concerning.
> >>
> >> On Fedora I have also to disable gcrypt, nettle and libiscsi because we
> >> don't have the statically linked libraries, but we don't need them with
> >> linux-user.
> > 
> > Do we actually try to link with those for linux-user? If not,
> > maybe we could make configure a bit more helpful about not
> > requiring you to manually --disable-foo them.
> 
> They are used by tests/qemu-iotests/socket_scm_helper and
> qemu-bridge-helper.
> 
> Perhaps we can disable qemu-iotests with linux-user only and to not
> build qemu-bridge-helper when tools are disabled?

Actually we should not build qemu-bridge-helper if all system emulator
builds are disabled, since its not intended as a standalone tool.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Peter Maydell]

On 5 June 2018 at 13:23, Laurent Vivier <laurent@vivier.eu> wrote:
> They are used by tests/qemu-iotests/socket_scm_helper and
> qemu-bridge-helper.
>
> Perhaps we can disable qemu-iotests with linux-user only and to not
> build qemu-bridge-helper when tools are disabled?

Yeah, we shouldn't be building those if you're using --disable-system
--disable-tools...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

Le 05/06/2018 à 14:33, Peter Maydell a écrit :
> On 5 June 2018 at 13:23, Laurent Vivier <laurent@vivier.eu> wrote:
>> They are used by tests/qemu-iotests/socket_scm_helper and
>> qemu-bridge-helper.
>>
>> Perhaps we can disable qemu-iotests with linux-user only and to not
>> build qemu-bridge-helper when tools are disabled?
> 
> Yeah, we shouldn't be building those if you're using --disable-system
> --disable-tools...

So something like that should do the work:

diff --git a/Makefile b/Makefile
....
+ifeq ($(CONFIG_SOFTMMU),y)
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
+endif
...
diff --git a/tests/Makefile.include b/tests/Makefile.include
...
+ifeq ($(CONFIG_SOFTMMU),y)
 QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) =
tests/qemu-iotests/socket_scm_helper$(EXESUF)
+endif
...

I prepare a patch...

Thanks,

Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Richard Henderson]

On 06/05/2018 03:52 AM, Peter Maydell wrote:
> On 5 June 2018 at 06:27, Richard Henderson <richard.henderson@linaro.org> wrote:
>> On Fedora 28, one can no longer build a static qemu.  We depend on libraries
>> for which Fedora no longer ships static versions.
> 
> Do you mean you can't build the system binaries statically (that's
> never really been very supported), or that you can't do a build
> with configure --disable-system --disable-tools --static ?

You're right that I can build with --disable-tools.
Although even there I have to also manually fiddle capstone.

I'm annoyed that "pkg-config --exists --static foo" does not really do what it
implies (to me at least), since Fedora does not ship the static library.


r~

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Peter Maydell]

On 5 June 2018 at 14:45, Richard Henderson <richard.henderson@linaro.org> wrote:
> I'm annoyed that "pkg-config --exists --static foo" does not really do what it
> implies (to me at least), since Fedora does not ship the static library.

Personally I would like to consider that a bug in the distro's
pkg-config files, though I think last time I suggested that
there was a contrary view.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Laurent Vivier]

Le 05/06/2018 à 07:27, Richard Henderson a écrit :
> On 06/03/2018 06:04 PM, Laurent Vivier wrote:
>> On 01/06/2018 00:49, Richard Henderson wrote:
>>> If the interp_prefix is a complete chroot, it may have a *lot* of files.
>>> Setting up the cache for this is quite expensive.
>>>
>>> For the most part, we can use the *at versions of various syscalls to
>>> attempt the operation in the prefix.  For the few cases that remain,
>>> attempt the operation in the prefix via concatenation and then retry
>>> if that fails.
>>>
>>
>> I like the idea, but it breaks real chroot.
>>
>> You can test it with:
>>
>> wget
>> https://github.com/vivier/linux-user-test-scrips/raw/master/create_chroot.sh
>>
>> then
>>
>> sudo sh ./create_chroot.sh /path/to/static/qemu-s390x stretch
> 
> *shrug* Works for me, at least as far as I can test.
> 
> Your script doesn't work outside debian, lacking debootstrap.
> At the moment, I can't build on debian *at all*.  Some bit of the build
> infrastructure is off and QEMU_FULL_VERSION gets incorrectly defined.  I have
> no idea how or why it is different than Fedora.

I had this problem on Fedora building in a separate directory, "make
distclean" at the top source directory fixed the problem for me.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Richard Henderson]

On 06/07/2018 01:01 AM, Laurent Vivier wrote:
>> Your script doesn't work outside debian, lacking debootstrap.
>> At the moment, I can't build on debian *at all*.  Some bit of the build
>> infrastructure is off and QEMU_FULL_VERSION gets incorrectly defined.  I have
>> no idea how or why it is different than Fedora.
> 
> I had this problem on Fedora building in a separate directory, "make
> distclean" at the top source directory fixed the problem for me.

Thanks.  I had never seen this failure mode before.


r~

Re: [Qemu-devel] [PATCH 6/6] linux-user: Use *at functions to implement interp_prefix

[Evan Green]

Apologies for getting lost in the dust storm, but where does this
leave this patch? Sounds like there's a real failure caused by it?
-Evan
On Thu, Jun 7, 2018 at 9:43 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 06/07/2018 01:01 AM, Laurent Vivier wrote:
> >> Your script doesn't work outside debian, lacking debootstrap.
> >> At the moment, I can't build on debian *at all*.  Some bit of the build
> >> infrastructure is off and QEMU_FULL_VERSION gets incorrectly defined.  I have
> >> no idea how or why it is different than Fedora.
> >
> > I had this problem on Fedora building in a separate directory, "make
> > distclean" at the top source directory fixed the problem for me.
>
> Thanks.  I had never seen this failure mode before.
>
>
> r~

Re: [Qemu-devel] [PATCH v5 0/6] linux-user: Reorg interp_prefix handling

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180531224911.23725-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH v5 0/6] linux-user: Reorg interp_prefix handling

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20180531224911.23725-1-richard.henderson@linaro.org -> patchew/20180531224911.23725-1-richard.henderson@linaro.org
Switched to a new branch 'test'
2755fa1986 linux-user: Use *at functions to implement interp_prefix
8af5f75489 linux-user: Check is_hostfd in mmap syscalls
83374c532a linux-user: Check contains_hostfd in select syscalls
41b80ec144 linux-user: Check is_hostfd in do_syscall
fb3aa3a0d8 linux-user: Add host_fds and manipulators
11567891c5 gdbstub: Return the fd from gdbserver_start

=== OUTPUT BEGIN ===
Checking PATCH 1/6: gdbstub: Return the fd from gdbserver_start...
Checking PATCH 2/6: linux-user: Add host_fds and manipulators...
Checking PATCH 3/6: linux-user: Check is_hostfd in do_syscall...
Checking PATCH 4/6: linux-user: Check contains_hostfd in select syscalls...
Checking PATCH 5/6: linux-user: Check is_hostfd in mmap syscalls...
Checking PATCH 6/6: linux-user: Use *at functions to implement interp_prefix...
ERROR: do not use assignment in if condition
#200: FILE: linux-user/syscall.c:8541:
+            if (!(fn = lock_user_string(arg2))) {

ERROR: do not use assignment in if condition
#218: FILE: linux-user/syscall.c:8562:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#236: FILE: linux-user/syscall.c:8577:
+        if (!(fn = lock_user_string(arg2))) {

ERROR: do not use assignment in if condition
#254: FILE: linux-user/syscall.c:8735:
+            if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#333: FILE: linux-user/syscall.c:9769:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#349: FILE: linux-user/syscall.c:9809:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#365: FILE: linux-user/syscall.c:10099:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#380: FILE: linux-user/syscall.c:10111:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#398: FILE: linux-user/syscall.c:11303:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#415: FILE: linux-user/syscall.c:11317:
+        if (!(fn = lock_user_string(arg1))) {

ERROR: do not use assignment in if condition
#433: FILE: linux-user/syscall.c:11349:
+        if (!(fn = lock_user_string(arg2))) {

ERROR: do not use assignment in if condition
#452: FILE: linux-user/syscall.c:12394:
+                if (!(fn = lock_user_string(arg2))) {

ERROR: do not use assignment in if condition
#472: FILE: linux-user/syscall.c:12433:
+        if (!(fn = lock_user_string(arg2))) {

total: 13 errors, 0 warnings, 423 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com