qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/4] linux-user patches for upstream
@ 2010-03-26 15:25 Riku Voipio
  2010-03-26 15:25 ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Riku Voipio
  0 siblings, 1 reply; 11+ messages in thread
From: Riku Voipio @ 2010-03-26 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio

From: Riku Voipio <riku.voipio@nokia.com>

Also available from:

git pull git://gitorious.org/qemu-maemo/qemu-maemo.git linux-user-for-upstream

Mostly trivial new syscalls, but more urgently fix the 0 sized write which is
a regression from Richard Henderson's multilevel page table patches.

Michael Casadevall (2):
  linux-user: add pselect syscall
  linux-user: Add the syscall id for pselect6 on ARM

Riku Voipio (2):
  Add inotify_init1 syscall support
  fix 0 sized write syscall

 configure                   |   18 ++++++
 exec.c                      |    5 +-
 linux-user/arm/syscall_nr.h |    2 +-
 linux-user/syscall.c        |  133 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 2 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall
  2010-03-26 15:25 [Qemu-devel] [PATCH 0/4] linux-user patches for upstream Riku Voipio
@ 2010-03-26 15:25 ` Riku Voipio
  2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
                     ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Riku Voipio @ 2010-03-26 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Michael Casadevall

From: Michael Casadevall <mcasadevall@ubuntu.com>

This patch adds support for the pselect syscall in linux-user emulation
and also adds several support functions required to translate the
timespec structs between the target and the host.

Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
Signed-off-by: Michael Casadevall <mcasadevall@ubuntu.com>
---
 linux-user/syscall.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 119 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 80d8633..845bb60 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -850,6 +850,38 @@ static inline abi_long copy_to_user_timeval(abi_ulong target_tv_addr,
     return 0;
 }
 
+static inline abi_long copy_from_user_timespec(struct timespec *ts,
+                                              abi_ulong target_ts_addr)
+{
+    struct target_timespec *target_ts;
+
+    if (!lock_user_struct(VERIFY_READ, target_ts, target_ts_addr, 1))
+        return -TARGET_EFAULT;
+
+    __get_user(ts->tv_sec, &target_ts->tv_sec);
+    __get_user(ts->tv_nsec, &target_ts->tv_nsec);
+
+    unlock_user_struct(target_ts, target_ts_addr, 0);
+
+    return 0;
+}
+
+
+static inline abi_long copy_to_user_timespec(abi_ulong target_ts_addr,
+                                            const struct timespec *ts)
+{
+    struct target_timespec *target_ts;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_ts, target_ts_addr, 0))
+        return -TARGET_EFAULT;
+
+    __put_user(ts->tv_sec, &target_ts->tv_sec);
+    __put_user(ts->tv_nsec, &target_ts->tv_nsec);
+
+    unlock_user_struct(target_ts, target_ts_addr, 1);
+
+    return 0;
+}
 #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
 #include <mqueue.h>
 
@@ -949,6 +981,75 @@ static abi_long do_select(int n,
     return ret;
 }
 
+#ifdef TARGET_NR_pselect6
+/* do_pselect() must return target values and target errnos. */
+static abi_long do_pselect(int n,
+                          abi_ulong rfd_addr, abi_ulong wfd_addr,
+                          abi_ulong efd_addr, abi_ulong target_tv_addr,
+                          abi_ulong set_addr)
+{
+    fd_set rfds, wfds, efds;
+    fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
+    struct timespec tv, *tv_ptr;
+    sigset_t set, *set_ptr;
+    abi_long ret;
+
+    if (rfd_addr) {
+        if (copy_from_user_fdset(&rfds, rfd_addr, n))
+            return -TARGET_EFAULT;
+        rfds_ptr = &rfds;
+    } else {
+        rfds_ptr = NULL;
+    }
+    if (wfd_addr) {
+        if (copy_from_user_fdset(&wfds, wfd_addr, n))
+            return -TARGET_EFAULT;
+        wfds_ptr = &wfds;
+    } else {
+        wfds_ptr = NULL;
+    }
+    if (efd_addr) {
+        if (copy_from_user_fdset(&efds, efd_addr, n))
+            return -TARGET_EFAULT;
+        efds_ptr = &efds;
+    } else {
+        efds_ptr = NULL;
+    }
+
+    if (target_tv_addr) {
+        if (copy_from_user_timespec(&tv, target_tv_addr))
+            return -TARGET_EFAULT;
+        tv_ptr = &tv;
+    } else {
+        tv_ptr = NULL;
+    }
+
+    /* We don't need to return sigmask to target */
+    if (set_addr) {
+        target_to_host_old_sigset(&set, &set_addr);
+        set_ptr = &set;
+    } else {
+        set_ptr = NULL;
+    }
+
+    ret = get_errno(pselect(n, rfds_ptr, wfds_ptr, efds_ptr, tv_ptr, set_ptr));
+
+    if (!is_error(ret)) {
+        if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n))
+            return -TARGET_EFAULT;
+        if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n))
+            return -TARGET_EFAULT;
+        if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
+            return -TARGET_EFAULT;
+
+        if (target_tv_addr && copy_to_user_timespec(target_tv_addr, &tv))
+            return -TARGET_EFAULT;
+    }
+
+    return ret;
+}
+#endif
+
 static abi_long do_pipe2(int host_pipe[], int flags)
 {
 #ifdef CONFIG_PIPE2
@@ -5186,6 +5287,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         }
         break;
 #endif
+
+#ifdef TARGET_NR_pselect6
+    case TARGET_NR_pselect6:
+        {
+            abi_ulong inp, outp, exp, tvp, set;
+            long nsel;
+
+            nsel = tswapl(arg1);
+            inp = tswapl(arg2);
+            outp = tswapl(arg3);
+            exp = tswapl(arg4);
+            tvp = tswapl(arg5);
+            set = tswapl(arg6);
+
+            ret = do_pselect(nsel, inp, outp, exp, tvp, set);
+        }
+        break;
+#endif
     case TARGET_NR_symlink:
         {
             void *p2;
-- 
1.6.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM
  2010-03-26 15:25 ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Riku Voipio
@ 2010-03-26 15:25   ` Riku Voipio
  2010-03-26 15:25     ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Riku Voipio
  2010-03-26 22:13     ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Aurelien Jarno
  2010-03-26 22:15   ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Aurelien Jarno
  2010-03-26 23:05   ` Paul Brook
  2 siblings, 2 replies; 11+ messages in thread
From: Riku Voipio @ 2010-03-26 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Michael Casadevall

From: Michael Casadevall <mcasadevall@ubuntu.com>

As this is now supported in newer linux kernels.

Signed-off-by: Michael Casadevall <mcasadevall@ubuntu.com>
Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
---
 linux-user/arm/syscall_nr.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
index b1db341..79a216a 100644
--- a/linux-user/arm/syscall_nr.h
+++ b/linux-user/arm/syscall_nr.h
@@ -338,7 +338,7 @@
 #define TARGET_NR_readlinkat			(332)
 #define TARGET_NR_fchmodat			(333)
 #define TARGET_NR_faccessat			(334)
-					/* 335 for pselect6 */
+#define TARGET_NR_pselect6			(335)
 					/* 336 for ppoll */
 #define TARGET_NR_unshare			(337)
 #define TARGET_NR_set_robust_list		(338)
-- 
1.6.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support
  2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
@ 2010-03-26 15:25     ` Riku Voipio
  2010-03-26 15:25       ` [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall Riku Voipio
  2010-03-26 22:14       ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Aurelien Jarno
  2010-03-26 22:13     ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Aurelien Jarno
  1 sibling, 2 replies; 11+ messages in thread
From: Riku Voipio @ 2010-03-26 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio

From: Riku Voipio <riku.voipio@nokia.com>

New syscall which gets actively used when you have a
fresh kernel.

Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
---
 configure            |   18 ++++++++++++++++++
 linux-user/syscall.c |   14 ++++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 6bc40a3..f9e08f6 100755
--- a/configure
+++ b/configure
@@ -1629,6 +1629,21 @@ if compile_prog "" "" ; then
   inotify=yes
 fi
 
+inotify1=no
+cat > $TMPC << EOF
+#include <sys/inotify.h>
+
+int
+main(void)
+{
+    /* try to start inotify */
+    return inotify_init1(0);
+}
+EOF
+if compile_prog "" "" ; then
+  inotify1=yes
+fi
+
 # check if utimensat and futimens are supported
 utimens=no
 cat > $TMPC << EOF
@@ -2136,6 +2151,9 @@ fi
 if test "$inotify" = "yes" ; then
   echo "CONFIG_INOTIFY=y" >> $config_host_mak
 fi
+if test "$inotify1" = "yes" ; then
+  echo "CONFIG_INOTIFY1=y" >> $config_host_mak
+fi
 if test "$byteswap_h" = "yes" ; then
   echo "CONFIG_BYTESWAP_H=y" >> $config_host_mak
 fi
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 845bb60..4b2a765 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -506,9 +506,18 @@ static int sys_inotify_rm_watch(int fd, int32_t wd)
   return (inotify_rm_watch(fd, wd));
 }
 #endif
+#ifdef CONFIG_INOTIFY1
+#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
+static int sys_inotify_init1(int flags)
+{
+  return (inotify_init1(flags));
+}
+#endif
+#endif
 #else
 /* Userspace can usually survive runtime without inotify */
 #undef TARGET_NR_inotify_init
+#undef TARGET_NR_inotify_init1
 #undef TARGET_NR_inotify_add_watch
 #undef TARGET_NR_inotify_rm_watch
 #endif /* CONFIG_INOTIFY  */
@@ -7174,6 +7183,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ret = get_errno(sys_inotify_init());
         break;
 #endif
+#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
+    case TARGET_NR_inotify_init1:
+        ret = get_errno(sys_inotify_init1(arg1));
+        break;
+#endif
 #if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
     case TARGET_NR_inotify_add_watch:
         p = lock_user_string(arg2);
-- 
1.6.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall
  2010-03-26 15:25     ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Riku Voipio
@ 2010-03-26 15:25       ` Riku Voipio
  2010-03-26 16:07         ` Richard Henderson
  2010-03-26 22:14       ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Aurelien Jarno
  1 sibling, 1 reply; 11+ messages in thread
From: Riku Voipio @ 2010-03-26 15:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: martin.mohring, Riku Voipio, Richard Henderson

From: Riku Voipio <riku.voipio@nokia.com>

fixes running ldconfig under qemu linux-user

Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: martin.mohring@opensuse.org
---
 exec.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index 14767b7..c834be2 100644
--- a/exec.c
+++ b/exec.c
@@ -2409,8 +2409,11 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
 #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
     assert(start < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
 #endif
+    if (len == 0) {
+        return 0;
+    }
 
-    if (start + len - 1 < start) {
+    if (start + len < start) {
         /* We've wrapped around.  */
         return -1;
     }
-- 
1.6.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall
  2010-03-26 15:25       ` [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall Riku Voipio
@ 2010-03-26 16:07         ` Richard Henderson
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2010-03-26 16:07 UTC (permalink / raw)
  To: Riku Voipio; +Cc: martin.mohring, Riku Voipio, qemu-devel

On 03/26/2010 08:25 AM, Riku Voipio wrote:
> +    if (len == 0) {
> +        return 0;
> +    }

This part is ok.

>  
> -    if (start + len - 1 < start) {
> +    if (start + len < start) {

This part re-introduces the bug I fixed.

  start = 0xffffff00
  len = 256

should succeed, and it won't reverting the (len-1) change.


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM
  2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
  2010-03-26 15:25     ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Riku Voipio
@ 2010-03-26 22:13     ` Aurelien Jarno
  1 sibling, 0 replies; 11+ messages in thread
From: Aurelien Jarno @ 2010-03-26 22:13 UTC (permalink / raw)
  To: Riku Voipio; +Cc: Michael Casadevall, Riku Voipio, qemu-devel

On Fri, Mar 26, 2010 at 03:25:10PM +0000, Riku Voipio wrote:
> From: Michael Casadevall <mcasadevall@ubuntu.com>
> 
> As this is now supported in newer linux kernels.
> 
> Signed-off-by: Michael Casadevall <mcasadevall@ubuntu.com>
> Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
> ---
>  linux-user/arm/syscall_nr.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
> index b1db341..79a216a 100644
> --- a/linux-user/arm/syscall_nr.h
> +++ b/linux-user/arm/syscall_nr.h
> @@ -338,7 +338,7 @@
>  #define TARGET_NR_readlinkat			(332)
>  #define TARGET_NR_fchmodat			(333)
>  #define TARGET_NR_faccessat			(334)
> -					/* 335 for pselect6 */
> +#define TARGET_NR_pselect6			(335)
>  					/* 336 for ppoll */
>  #define TARGET_NR_unshare			(337)
>  #define TARGET_NR_set_robust_list		(338)
> -- 
> 1.6.5
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support
  2010-03-26 15:25     ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Riku Voipio
  2010-03-26 15:25       ` [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall Riku Voipio
@ 2010-03-26 22:14       ` Aurelien Jarno
  1 sibling, 0 replies; 11+ messages in thread
From: Aurelien Jarno @ 2010-03-26 22:14 UTC (permalink / raw)
  To: Riku Voipio; +Cc: Riku Voipio, qemu-devel

On Fri, Mar 26, 2010 at 03:25:11PM +0000, Riku Voipio wrote:
> From: Riku Voipio <riku.voipio@nokia.com>
> 
> New syscall which gets actively used when you have a
> fresh kernel.
> 
> Signed-off-by: Riku Voipio <riku.voipio@nokia.com>

Thanks, applied.

> ---
>  configure            |   18 ++++++++++++++++++
>  linux-user/syscall.c |   14 ++++++++++++++
>  2 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index 6bc40a3..f9e08f6 100755
> --- a/configure
> +++ b/configure
> @@ -1629,6 +1629,21 @@ if compile_prog "" "" ; then
>    inotify=yes
>  fi
>  
> +inotify1=no
> +cat > $TMPC << EOF
> +#include <sys/inotify.h>
> +
> +int
> +main(void)
> +{
> +    /* try to start inotify */
> +    return inotify_init1(0);
> +}
> +EOF
> +if compile_prog "" "" ; then
> +  inotify1=yes
> +fi
> +
>  # check if utimensat and futimens are supported
>  utimens=no
>  cat > $TMPC << EOF
> @@ -2136,6 +2151,9 @@ fi
>  if test "$inotify" = "yes" ; then
>    echo "CONFIG_INOTIFY=y" >> $config_host_mak
>  fi
> +if test "$inotify1" = "yes" ; then
> +  echo "CONFIG_INOTIFY1=y" >> $config_host_mak
> +fi
>  if test "$byteswap_h" = "yes" ; then
>    echo "CONFIG_BYTESWAP_H=y" >> $config_host_mak
>  fi
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 845bb60..4b2a765 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -506,9 +506,18 @@ static int sys_inotify_rm_watch(int fd, int32_t wd)
>    return (inotify_rm_watch(fd, wd));
>  }
>  #endif
> +#ifdef CONFIG_INOTIFY1
> +#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
> +static int sys_inotify_init1(int flags)
> +{
> +  return (inotify_init1(flags));
> +}
> +#endif
> +#endif
>  #else
>  /* Userspace can usually survive runtime without inotify */
>  #undef TARGET_NR_inotify_init
> +#undef TARGET_NR_inotify_init1
>  #undef TARGET_NR_inotify_add_watch
>  #undef TARGET_NR_inotify_rm_watch
>  #endif /* CONFIG_INOTIFY  */
> @@ -7174,6 +7183,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          ret = get_errno(sys_inotify_init());
>          break;
>  #endif
> +#if defined(TARGET_NR_inotify_init1) && defined(__NR_inotify_init1)
> +    case TARGET_NR_inotify_init1:
> +        ret = get_errno(sys_inotify_init1(arg1));
> +        break;
> +#endif
>  #if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
>      case TARGET_NR_inotify_add_watch:
>          p = lock_user_string(arg2);
> -- 
> 1.6.5
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall
  2010-03-26 15:25 ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Riku Voipio
  2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
@ 2010-03-26 22:15   ` Aurelien Jarno
  2010-03-26 23:05   ` Paul Brook
  2 siblings, 0 replies; 11+ messages in thread
From: Aurelien Jarno @ 2010-03-26 22:15 UTC (permalink / raw)
  To: Riku Voipio; +Cc: Michael Casadevall, Riku Voipio, qemu-devel

On Fri, Mar 26, 2010 at 03:25:09PM +0000, Riku Voipio wrote:
> From: Michael Casadevall <mcasadevall@ubuntu.com>
> 
> This patch adds support for the pselect syscall in linux-user emulation
> and also adds several support functions required to translate the
> timespec structs between the target and the host.
> 
> Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
> Signed-off-by: Michael Casadevall <mcasadevall@ubuntu.com>
> ---
>  linux-user/syscall.c |  119 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 119 insertions(+), 0 deletions(-)

This patch basically needs ok, but need to conform to CODING_STYLE (see
below).

> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 80d8633..845bb60 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -850,6 +850,38 @@ static inline abi_long copy_to_user_timeval(abi_ulong target_tv_addr,
>      return 0;
>  }
>  
> +static inline abi_long copy_from_user_timespec(struct timespec *ts,
> +                                              abi_ulong target_ts_addr)
> +{
> +    struct target_timespec *target_ts;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_ts, target_ts_addr, 1))
> +        return -TARGET_EFAULT;

You should use curly braces around the return;

> +
> +    __get_user(ts->tv_sec, &target_ts->tv_sec);
> +    __get_user(ts->tv_nsec, &target_ts->tv_nsec);
> +
> +    unlock_user_struct(target_ts, target_ts_addr, 0);
> +
> +    return 0;
> +}
> +
> +
> +static inline abi_long copy_to_user_timespec(abi_ulong target_ts_addr,
> +                                            const struct timespec *ts)
> +{
> +    struct target_timespec *target_ts;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_ts, target_ts_addr, 0))
> +        return -TARGET_EFAULT;
> +
> +    __put_user(ts->tv_sec, &target_ts->tv_sec);
> +    __put_user(ts->tv_nsec, &target_ts->tv_nsec);
> +
> +    unlock_user_struct(target_ts, target_ts_addr, 1);
> +
> +    return 0;
> +}
>  #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
>  #include <mqueue.h>
>  
> @@ -949,6 +981,75 @@ static abi_long do_select(int n,
>      return ret;
>  }
>  
> +#ifdef TARGET_NR_pselect6
> +/* do_pselect() must return target values and target errnos. */
> +static abi_long do_pselect(int n,
> +                          abi_ulong rfd_addr, abi_ulong wfd_addr,
> +                          abi_ulong efd_addr, abi_ulong target_tv_addr,
> +                          abi_ulong set_addr)
> +{
> +    fd_set rfds, wfds, efds;
> +    fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
> +    struct timespec tv, *tv_ptr;
> +    sigset_t set, *set_ptr;
> +    abi_long ret;
> +
> +    if (rfd_addr) {
> +        if (copy_from_user_fdset(&rfds, rfd_addr, n))
> +            return -TARGET_EFAULT;

Same here, and later on.

> +        rfds_ptr = &rfds;
> +    } else {
> +        rfds_ptr = NULL;
> +    }
> +    if (wfd_addr) {
> +        if (copy_from_user_fdset(&wfds, wfd_addr, n))
> +            return -TARGET_EFAULT;
> +        wfds_ptr = &wfds;
> +    } else {
> +        wfds_ptr = NULL;
> +    }
> +    if (efd_addr) {
> +        if (copy_from_user_fdset(&efds, efd_addr, n))
> +            return -TARGET_EFAULT;
> +        efds_ptr = &efds;
> +    } else {
> +        efds_ptr = NULL;
> +    }
> +
> +    if (target_tv_addr) {
> +        if (copy_from_user_timespec(&tv, target_tv_addr))
> +            return -TARGET_EFAULT;
> +        tv_ptr = &tv;
> +    } else {
> +        tv_ptr = NULL;
> +    }
> +
> +    /* We don't need to return sigmask to target */
> +    if (set_addr) {
> +        target_to_host_old_sigset(&set, &set_addr);
> +        set_ptr = &set;
> +    } else {
> +        set_ptr = NULL;
> +    }
> +
> +    ret = get_errno(pselect(n, rfds_ptr, wfds_ptr, efds_ptr, tv_ptr, set_ptr));
> +
> +    if (!is_error(ret)) {
> +        if (rfd_addr && copy_to_user_fdset(rfd_addr, &rfds, n))
> +            return -TARGET_EFAULT;
> +        if (wfd_addr && copy_to_user_fdset(wfd_addr, &wfds, n))
> +            return -TARGET_EFAULT;
> +        if (efd_addr && copy_to_user_fdset(efd_addr, &efds, n))
> +            return -TARGET_EFAULT;
> +
> +        if (target_tv_addr && copy_to_user_timespec(target_tv_addr, &tv))
> +            return -TARGET_EFAULT;
> +    }
> +
> +    return ret;
> +}
> +#endif
> +
>  static abi_long do_pipe2(int host_pipe[], int flags)
>  {
>  #ifdef CONFIG_PIPE2
> @@ -5186,6 +5287,24 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>          }
>          break;
>  #endif
> +
> +#ifdef TARGET_NR_pselect6
> +    case TARGET_NR_pselect6:
> +        {
> +            abi_ulong inp, outp, exp, tvp, set;
> +            long nsel;
> +
> +            nsel = tswapl(arg1);
> +            inp = tswapl(arg2);
> +            outp = tswapl(arg3);
> +            exp = tswapl(arg4);
> +            tvp = tswapl(arg5);
> +            set = tswapl(arg6);
> +
> +            ret = do_pselect(nsel, inp, outp, exp, tvp, set);
> +        }
> +        break;
> +#endif
>      case TARGET_NR_symlink:
>          {
>              void *p2;
> -- 
> 1.6.5
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall
  2010-03-26 15:25 ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Riku Voipio
  2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
  2010-03-26 22:15   ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Aurelien Jarno
@ 2010-03-26 23:05   ` Paul Brook
  2010-03-28 18:49     ` Jamie Lokier
  2 siblings, 1 reply; 11+ messages in thread
From: Paul Brook @ 2010-03-26 23:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Riku Voipio, Michael Casadevall

>This patch adds support for the pselect syscall in linux-user emulation
>and also adds several support functions required to translate the
>timespec structs between the target and the host.

IIUC the whole point of the pselect is that it should be atomic. By emulating 
this in a non-atomic fasion I think you're re-introducing the race condition 
that it is designed to avoid.

Wouldn't it be better to just return ENOSYS and let the guest deal with the 
problem?

Paul

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall
  2010-03-26 23:05   ` Paul Brook
@ 2010-03-28 18:49     ` Jamie Lokier
  0 siblings, 0 replies; 11+ messages in thread
From: Jamie Lokier @ 2010-03-28 18:49 UTC (permalink / raw)
  To: Paul Brook; +Cc: Michael Casadevall, Riku Voipio, Riku Voipio, qemu-devel

Paul Brook wrote:
> >This patch adds support for the pselect syscall in linux-user emulation
> >and also adds several support functions required to translate the
> >timespec structs between the target and the host.
> 
> IIUC the whole point of the pselect is that it should be atomic. By
> emulating this in a non-atomic fasion I think you're re-introducing
> the race condition that it is designed to avoid.
>
> Wouldn't it be better to just return ENOSYS and let the guest deal with the 
> problem?

Imho, it's very important to return ENOSYS if the atomic behaviour is
not provided.

The patch actually calls the host's pselect() - it looks almost ok...

But actually, host's pselect() may be Glibc or some other equally
buggy libc, which "emulates" pselect wrongly.  FreeBSD's pselect has
the same problem going back years - with a very recent patch to
support it in the kernel.  It's not just Glibc.

Careful apps detect and avoid libc's bug by calling the pselect syscall
directly.  If it returns ENOSYS, they just use a different (but
slightly slower) race-free technique, or abort - at least they don't
have a quiet corruption bug.

But here we're providing the kernel syscall to guest apps.  There's no
way for them to detect if it's really atomic or not.  And the ones
written carefully to avoid buggy libc will wrongly use it, because
they think it's a kernel call.

I think it's really important we don't pass on libc's buggy behaviour
to the emulated kernel interface, to even break careful apps.

Solution, Riku: Instead of calling pselect(...), qemu should call
syscall(SYS_pselect, ...), with a comment like:

    /*
     * Glibc and some other libcs unhelpfully "emulate" pselect()
     * using select() when the real system call isn't available, and
     * break the atomic guarantee which is the entire point of
     * pselect().  To avoid these libc bugs, go straight to the host
     * kernel using syscall().  If the host kernel doesn't support
     * pselect() it will return ENOSYS, which is what we want to
     * return to the guest.
     */

There might need to be some header file included to get syscall() and
SYS_pselect, which sometimes has other names like NR_pselect.  And
unfortunately the call arguments aren't quite the same, at least on
Linux.

Same goes for ppoll() if that is ever added.

-- Jamie

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-03-28 18:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-26 15:25 [Qemu-devel] [PATCH 0/4] linux-user patches for upstream Riku Voipio
2010-03-26 15:25 ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Riku Voipio
2010-03-26 15:25   ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Riku Voipio
2010-03-26 15:25     ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Riku Voipio
2010-03-26 15:25       ` [Qemu-devel] [PATCH 4/4] fix 0 sized write syscall Riku Voipio
2010-03-26 16:07         ` Richard Henderson
2010-03-26 22:14       ` [Qemu-devel] [PATCH 3/4] Add inotify_init1 syscall support Aurelien Jarno
2010-03-26 22:13     ` [Qemu-devel] [PATCH 2/4] linux-user: Add the syscall id for pselect6 on ARM Aurelien Jarno
2010-03-26 22:15   ` [Qemu-devel] [PATCH 1/4] linux-user: add pselect syscall Aurelien Jarno
2010-03-26 23:05   ` Paul Brook
2010-03-28 18:49     ` Jamie Lokier

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).