[Qemu-devel] [PATCH 00/18] linux-user: Extend safe_syscall wrapper use

[Qemu-devel] [PATCH 00/18] linux-user: Extend safe_syscall wrapper use

[Peter Maydell]

This set of pretty dull patches extends the use of the safe_syscall
wrapper to every syscall listed in the signal(7) manpage as being
interruptible.

Most of the patches are just straightforward "use the wrapper" changes.
For a few things get a little more complicated because we need to use
the direct syscall rather than the helpful libc wrapper:
 * for the IPC syscalls we need to handle the host kernel maybe
   doing these via individual syscalls and maybe via the single
   'ipc' syscall
 * poll has to be implemented via the ppoll syscall now, which means
   converting the timeout argument
 * in order to have all the fcntl-related syscalls go via fcntl64
   (rather than using a wrapper for both fcntl and fcntl64 on 32-bit
   hosts), there are patches which clean up the conversion of the
   target_flock data structures. These in passing fix buggy conversion
   code which was making us fail some LTP tests

The last two patches are trivial ones which neaten up the QEMU
strace output for the cases where the returned errnos printed by
the strace layer are QEMU-internal ones rather than real guest errnos.

This patchset sits on top of:
 * the 'fix various signal race conditions' patchset currently on list:
   https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg05057.html
 * the fadvise patches (on list, reviewed)
 * the 'provide frame information in x86-64 safe_syscall' patch v2
   (on list, reviewed)

https://git.linaro.org/people/peter.maydell/qemu-arm.git sigrace-fixes-3
is a git branch with those prequisites plus this patchset.

thanks
-- PMM

Peter Maydell (18):
  linux-user: Use safe_syscall wrapper for readv and writev syscalls
  linux-user: Use safe_syscall wrapper for connect syscall
  linux-user: Use safe_syscall wrapper for send* and recv* syscalls
  linux-user: Use safe_syscall wrapper for msgsnd and msgrcv
  linux-user: Use safe_syscall wrapper for mq_timedsend and
    mq_timedreceive
  linux-user: Use safe_syscall wrapper for flock
  linux-user: Use safe_syscall wrapper for rt_sigtimedwait syscall
  linux-user: Use safe_syscall wrapper for sleep syscalls
  linux-user: Use safe_syscall wrapper for poll and ppoll syscalls
  linux-user: Use safe_syscall wrapper for epoll_wait syscalls
  linux-user: Use safe_syscall wrapper for semop
  linux-user: Use safe_syscall wrapper for accept and accept4 syscalls
  linux-user: Use safe_syscall wrapper for ioctl
  linux-user: Use __get_user() and __put_user() to handle structs in
    do_fcntl()
  linux-user: Correct signedness of target_flock l_start and l_len
    fields
  linux-user: Use safe_syscall wrapper for fcntl
  linux-user: Make target_strerror() return 'const char *'
  linux-user: Special-case ERESTARTSYS in target_strerror()

 configure                 |  21 +-
 linux-user/qemu.h         |   2 +-
 linux-user/strace.c       |   4 +-
 linux-user/syscall.c      | 547 ++++++++++++++++++++++++++++------------------
 linux-user/syscall_defs.h |  34 +--
 5 files changed, 359 insertions(+), 249 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 01/18] linux-user: Use safe_syscall wrapper for readv and writev syscalls

[Peter Maydell]

Use the safe_syscall wrapper for readv and writev syscalls.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 31a9484..28f7244 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -699,6 +699,8 @@ safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
 safe_syscall2(int, kill, pid_t, pid, int, sig)
 safe_syscall2(int, tkill, int, tid, int, sig)
 safe_syscall3(int, tgkill, int, tgid, int, pid, int, sig)
+safe_syscall3(ssize_t, readv, int, fd, const struct iovec *, iov, int, iovcnt)
+safe_syscall3(ssize_t, writev, int, fd, const struct iovec *, iov, int, iovcnt)
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -8345,7 +8347,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
             if (vec != NULL) {
-                ret = get_errno(readv(arg1, vec, arg3));
+                ret = get_errno(safe_readv(arg1, vec, arg3));
                 unlock_iovec(vec, arg2, arg3, 1);
             } else {
                 ret = -host_to_target_errno(errno);
@@ -8356,7 +8358,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1);
             if (vec != NULL) {
-                ret = get_errno(writev(arg1, vec, arg3));
+                ret = get_errno(safe_writev(arg1, vec, arg3));
                 unlock_iovec(vec, arg2, arg3, 0);
             } else {
                 ret = -host_to_target_errno(errno);
-- 
1.9.1

[Qemu-devel] [PATCH 02/18] linux-user: Use safe_syscall wrapper for connect syscall

[Peter Maydell]

Use the safe_syscall wrapper for the connect syscall.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 28f7244..b649a8c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -701,6 +701,8 @@ safe_syscall2(int, tkill, int, tid, int, sig)
 safe_syscall3(int, tgkill, int, tgid, int, pid, int, sig)
 safe_syscall3(ssize_t, readv, int, fd, const struct iovec *, iov, int, iovcnt)
 safe_syscall3(ssize_t, writev, int, fd, const struct iovec *, iov, int, iovcnt)
+safe_syscall3(int, connect, int, fd, const struct sockaddr *, addr,
+              socklen_t, addrlen)
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -2236,7 +2238,7 @@ static abi_long do_connect(int sockfd, abi_ulong target_addr,
     if (ret)
         return ret;
 
-    return get_errno(connect(sockfd, addr, addrlen));
+    return get_errno(safe_connect(sockfd, addr, addrlen));
 }
 
 /* do_sendrecvmsg_locked() Must return target values and target errnos. */
-- 
1.9.1

[Qemu-devel] [PATCH 03/18] linux-user: Use safe_syscall wrapper for send* and recv* syscalls

[Peter Maydell]

Use the safe_syscall wrapper for the send, sendto, sendmsg, recv,
recvfrom and recvmsg syscalls.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b649a8c..f5c5476 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -703,6 +703,12 @@ safe_syscall3(ssize_t, readv, int, fd, const struct iovec *, iov, int, iovcnt)
 safe_syscall3(ssize_t, writev, int, fd, const struct iovec *, iov, int, iovcnt)
 safe_syscall3(int, connect, int, fd, const struct sockaddr *, addr,
               socklen_t, addrlen)
+safe_syscall6(ssize_t, sendto, int, fd, const void *, buf, size_t, len,
+              int, flags, const struct sockaddr *, addr, socklen_t, addrlen)
+safe_syscall6(ssize_t, recvfrom, int, fd, void *, buf, size_t, len,
+              int, flags, struct sockaddr *, addr, socklen_t *, addrlen)
+safe_syscall3(ssize_t, sendmsg, int, fd, const struct msghdr *, msg, int, flags)
+safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -2282,9 +2288,9 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp,
     if (send) {
         ret = target_to_host_cmsg(&msg, msgp);
         if (ret == 0)
-            ret = get_errno(sendmsg(fd, &msg, flags));
+            ret = get_errno(safe_sendmsg(fd, &msg, flags));
     } else {
-        ret = get_errno(recvmsg(fd, &msg, flags));
+        ret = get_errno(safe_recvmsg(fd, &msg, flags));
         if (!is_error(ret)) {
             len = ret;
             ret = host_to_target_cmsg(msgp, &msg);
@@ -2521,9 +2527,9 @@ static abi_long do_sendto(int fd, abi_ulong msg, size_t len, int flags,
             unlock_user(host_msg, msg, 0);
             return ret;
         }
-        ret = get_errno(sendto(fd, host_msg, len, flags, addr, addrlen));
+        ret = get_errno(safe_sendto(fd, host_msg, len, flags, addr, addrlen));
     } else {
-        ret = get_errno(send(fd, host_msg, len, flags));
+        ret = get_errno(safe_sendto(fd, host_msg, len, flags, NULL, 0));
     }
     unlock_user(host_msg, msg, 0);
     return ret;
@@ -2552,10 +2558,11 @@ static abi_long do_recvfrom(int fd, abi_ulong msg, size_t len, int flags,
             goto fail;
         }
         addr = alloca(addrlen);
-        ret = get_errno(recvfrom(fd, host_msg, len, flags, addr, &addrlen));
+        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags,
+                                      addr, &addrlen));
     } else {
         addr = NULL; /* To keep compiler quiet.  */
-        ret = get_errno(qemu_recv(fd, host_msg, len, flags));
+        ret = get_errno(safe_recvfrom(fd, host_msg, len, flags, NULL, 0));
     }
     if (!is_error(ret)) {
         if (target_addr) {
-- 
1.9.1

[Qemu-devel] [PATCH 04/18] linux-user: Use safe_syscall wrapper for msgsnd and msgrcv

[Peter Maydell]

Use the safe_syscall wrapper for msgsnd and msgrcv syscalls.
This is made slightly awkward by some host architectures providing
only a single 'ipc' syscall rather than separate syscalls per
operation; we provide safe_msgsnd() and safe_msgrcv() as wrappers
around safe_ipc() to handle this if needed.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f5c5476..3735d7d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -709,6 +709,34 @@ safe_syscall6(ssize_t, recvfrom, int, fd, void *, buf, size_t, len,
               int, flags, struct sockaddr *, addr, socklen_t *, addrlen)
 safe_syscall3(ssize_t, sendmsg, int, fd, const struct msghdr *, msg, int, flags)
 safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
+#ifdef __NR_msgsnd
+safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
+              int, flags)
+safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
+              long, msgtype, int, flags)
+#else
+/* This host kernel architecture uses a single ipc syscall; fake up
+ * wrappers for the sub-operations to hide this implementation detail.
+ * Annoyingly we can't include linux/ipc.h to get the constant definitions
+ * for the call parameter because some structs in there conflict with the
+ * sys/ipc.h ones. So we just define them here, and rely on them being
+ * the same for all host architectures.
+ */
+#define Q_MSGSND 11
+#define Q_MSGRCV 12
+#define Q_IPCCALL(VERSION, OP) ((VERSION) << 16 | (OP))
+
+safe_syscall6(int, ipc, int, call, long, first, long, second, long, third,
+              void *, ptr, long, fifth)
+static int safe_msgsnd(int msgid, const void *msgp, size_t sz, int flags)
+{
+    return safe_ipc(Q_IPCCALL(0, Q_MSGSND), msgid, sz, flags, (void *)msgp, 0);
+}
+static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
+{
+    return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
+}
+#endif
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -3155,7 +3183,7 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
     }
     host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
     memcpy(host_mb->mtext, target_mb->mtext, msgsz);
-    ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
+    ret = get_errno(safe_msgsnd(msqid, host_mb, msgsz, msgflg));
     g_free(host_mb);
     unlock_user_struct(target_mb, msgp, 0);
 
@@ -3183,7 +3211,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
         ret = -TARGET_ENOMEM;
         goto end;
     }
-    ret = get_errno(msgrcv(msqid, host_mb, msgsz, msgtyp, msgflg));
+    ret = get_errno(safe_msgrcv(msqid, host_mb, msgsz, msgtyp, msgflg));
 
     if (ret > 0) {
         abi_ulong target_mtext_addr = msgp + sizeof(abi_ulong);
-- 
1.9.1

[Qemu-devel] [PATCH 05/18] linux-user: Use safe_syscall wrapper for mq_timedsend and mq_timedreceive

[Peter Maydell]

Use the safe_syscall wrapper for mq_timedsend and mq_timedreceive syscalls.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3735d7d..cdc7428 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -737,6 +737,12 @@ static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
     return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
 }
 #endif
+#if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
+safe_syscall5(int, mq_timedsend, int, mqdes, const char *, msg_ptr,
+              size_t, len, unsigned, prio, const struct timespec *, timeout)
+safe_syscall5(int, mq_timedreceive, int, mqdes, char *, msg_ptr,
+              size_t, len, unsigned *, prio, const struct timespec *, timeout)
+#endif
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -9952,11 +9958,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             p = lock_user (VERIFY_READ, arg2, arg3, 1);
             if (arg5 != 0) {
                 target_to_host_timespec(&ts, arg5);
-                ret = get_errno(mq_timedsend(arg1, p, arg3, arg4, &ts));
+                ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, &ts));
                 host_to_target_timespec(arg5, &ts);
+            } else {
+                ret = get_errno(safe_mq_timedsend(arg1, p, arg3, arg4, NULL));
             }
-            else
-                ret = get_errno(mq_send(arg1, p, arg3, arg4));
             unlock_user (p, arg2, arg3);
         }
         break;
@@ -9969,11 +9975,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             p = lock_user (VERIFY_READ, arg2, arg3, 1);
             if (arg5 != 0) {
                 target_to_host_timespec(&ts, arg5);
-                ret = get_errno(mq_timedreceive(arg1, p, arg3, &prio, &ts));
+                ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
+                                                     &prio, &ts));
                 host_to_target_timespec(arg5, &ts);
+            } else {
+                ret = get_errno(safe_mq_timedreceive(arg1, p, arg3,
+                                                     &prio, NULL));
             }
-            else
-                ret = get_errno(mq_receive(arg1, p, arg3, &prio));
             unlock_user (p, arg2, arg3);
             if (arg4 != 0)
                 put_user_u32(prio, arg4);
-- 
1.9.1

[Qemu-devel] [PATCH 06/18] linux-user: Use safe_syscall wrapper for flock

[Peter Maydell]

Use the safe_syscall wrapper for the flock syscall.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index cdc7428..559b4f7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -709,6 +709,7 @@ safe_syscall6(ssize_t, recvfrom, int, fd, void *, buf, size_t, len,
               int, flags, struct sockaddr *, addr, socklen_t *, addrlen)
 safe_syscall3(ssize_t, sendmsg, int, fd, const struct msghdr *, msg, int, flags)
 safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
+safe_syscall2(int, flock, int, fd, int, operation)
 #ifdef __NR_msgsnd
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
@@ -8384,7 +8385,7 @@ 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 */
-        ret = get_errno(flock(arg1, arg2));
+        ret = get_errno(safe_flock(arg1, arg2));
         break;
     case TARGET_NR_readv:
         {
-- 
1.9.1

[Qemu-devel] [PATCH 07/18] linux-user: Use safe_syscall wrapper for rt_sigtimedwait syscall

[Peter Maydell]

Use the safe_syscall wrapper for the rt_sigtimedwait syscall.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 559b4f7..300edc4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -710,6 +710,8 @@ safe_syscall6(ssize_t, recvfrom, int, fd, void *, buf, size_t, len,
 safe_syscall3(ssize_t, sendmsg, int, fd, const struct msghdr *, msg, int, flags)
 safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
 safe_syscall2(int, flock, int, fd, int, operation)
+safe_syscall4(int, rt_sigtimedwait, const sigset_t *, these, siginfo_t *, uinfo,
+              const struct timespec *, uts, size_t, sigsetsize)
 #ifdef __NR_msgsnd
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
@@ -7110,7 +7112,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             } else {
                 puts = NULL;
             }
-            ret = get_errno(sigtimedwait(&set, &uinfo, puts));
+            ret = get_errno(safe_rt_sigtimedwait(&set, &uinfo, puts,
+                                                 SIGSET_T_SIZE));
             if (!is_error(ret)) {
                 if (arg2) {
                     p = lock_user(VERIFY_WRITE, arg2, sizeof(target_siginfo_t),
-- 
1.9.1

[Qemu-devel] [PATCH 08/18] linux-user: Use safe_syscall wrapper for sleep syscalls

[Peter Maydell]

Use the safe_syscall wrapper for the clock_nanosleep and nanosleep
syscalls.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 300edc4..c87665a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -712,6 +712,12 @@ safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
 safe_syscall2(int, flock, int, fd, int, operation)
 safe_syscall4(int, rt_sigtimedwait, const sigset_t *, these, siginfo_t *, uinfo,
               const struct timespec *, uts, size_t, sigsetsize)
+safe_syscall2(int, nanosleep, const struct timespec *, req,
+              struct timespec *, rem)
+#ifdef TARGET_NR_clock_nanosleep
+safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
+              const struct timespec *, req, struct timespec *, rem)
+#endif
 #ifdef __NR_msgsnd
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
@@ -8564,7 +8570,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct timespec req, rem;
             target_to_host_timespec(&req, arg1);
-            ret = get_errno(nanosleep(&req, &rem));
+            ret = get_errno(safe_nanosleep(&req, &rem));
             if (is_error(ret) && arg2) {
                 host_to_target_timespec(arg2, &rem);
             }
@@ -9832,14 +9838,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
         struct timespec ts;
         target_to_host_timespec(&ts, arg3);
-        ret = get_errno(clock_nanosleep(arg1, arg2, &ts, arg4 ? &ts : NULL));
+        ret = get_errno(safe_clock_nanosleep(arg1, arg2,
+                                             &ts, arg4 ? &ts : NULL));
         if (arg4)
             host_to_target_timespec(arg4, &ts);
 
 #if defined(TARGET_PPC)
         /* clock_nanosleep is odd in that it returns positive errno values.
          * On PPC, CR0 bit 3 should be set in such a situation. */
-        if (ret) {
+        if (ret && ret != -TARGET_ERESTARTSYS) {
             ((CPUPPCState *)cpu_env)->crf[0] |= 1;
         }
 #endif
-- 
1.9.1

[Qemu-devel] [PATCH 09/18] linux-user: Use safe_syscall wrapper for poll and ppoll syscalls

[Peter Maydell]

Use the safe_syscall wrapper for the poll and ppoll syscalls.
Since not all host architectures will have a poll syscall, we
have to rewrite the TARGET_NR_poll handling to use ppoll instead
(we can assume everywhere has ppoll by now).

We take the opportunity to switch to the code structure
already used in the implementation of epoll_wait and epoll_pwait,
which uses a switch() to avoid interleaving #if and if (),
and to stop using a variable with a leading '_' which is in
the implementation's namespace.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c87665a..f1cf809 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -416,16 +416,6 @@ static int sys_inotify_init1(int flags)
 #undef TARGET_NR_inotify_rm_watch
 #endif /* CONFIG_INOTIFY  */
 
-#if defined(TARGET_NR_ppoll)
-#ifndef __NR_ppoll
-# define __NR_ppoll -1
-#endif
-#define __NR_sys_ppoll __NR_ppoll
-_syscall5(int, sys_ppoll, struct pollfd *, fds, nfds_t, nfds,
-          struct timespec *, timeout, const sigset_t *, sigmask,
-          size_t, sigsetsize)
-#endif
-
 #if defined(TARGET_NR_prlimit64)
 #ifndef __NR_prlimit64
 # define __NR_prlimit64 -1
@@ -693,6 +683,9 @@ safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, siginfo_t *, infop, \
 safe_syscall3(int, execve, const char *, filename, char **, argv, char **, envp)
 safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \
               fd_set *, exceptfds, struct timespec *, timeout, void *, sig)
+safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
+              struct timespec *, tsp, const sigset_t *, sigmask,
+              size_t, sigsetsize)
 safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \
               const struct timespec *,timeout,int *,uaddr2,int,val3)
 safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
@@ -8323,7 +8316,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct target_pollfd *target_pfd;
             unsigned int nfds = arg2;
-            int timeout = arg3;
             struct pollfd *pfd;
             unsigned int i;
 
@@ -8343,8 +8335,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 }
             }
 
+            switch (num) {
 # ifdef TARGET_NR_ppoll
-            if (num == TARGET_NR_ppoll) {
+            case TARGET_NR_ppoll:
+            {
                 struct timespec _timeout_ts, *timeout_ts = &_timeout_ts;
                 target_sigset_t *target_set;
                 sigset_t _set, *set = &_set;
@@ -8369,8 +8363,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                     set = NULL;
                 }
 
-                ret = get_errno(sys_ppoll(pfd, nfds, timeout_ts,
-                                          set, SIGSET_T_SIZE));
+                ret = get_errno(safe_ppoll(pfd, nfds, timeout_ts,
+                                           set, SIGSET_T_SIZE));
 
                 if (!is_error(ret) && arg3) {
                     host_to_target_timespec(arg3, timeout_ts);
@@ -8378,9 +8372,30 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 if (arg4) {
                     unlock_user(target_set, arg4, 0);
                 }
-            } else
+                break;
+            }
 # endif
-                ret = get_errno(poll(pfd, nfds, timeout));
+# ifdef TARGET_NR_poll
+            case TARGET_NR_poll:
+            {
+                struct timespec ts, *pts;
+
+                if (arg3 >= 0) {
+                    /* Convert ms to secs, ns */
+                    ts.tv_sec = arg3 / 1000;
+                    ts.tv_nsec = (arg3 % 1000) * 1000000LL;
+                    pts = &ts;
+                } else {
+                    /* -ve poll() timeout means "infinite" */
+                    pts = NULL;
+                }
+                ret = get_errno(safe_ppoll(pfd, nfds, pts, NULL, 0));
+                break;
+            }
+# endif
+            default:
+                g_assert_not_reached();
+            }
 
             if (!is_error(ret)) {
                 for(i = 0; i < nfds; i++) {
-- 
1.9.1

[Qemu-devel] [PATCH 10/18] linux-user: Use safe_syscall wrapper for epoll_wait syscalls

[Peter Maydell]

Use the safe_syscall wrapper for epoll_wait and epoll_pwait syscalls.

Since we now directly use the host epoll_pwait syscall for both
epoll_wait and epoll_pwait, we don't need the configure machinery
to check whether glibc supports epoll_pwait(). (The kernel has
supported the syscall since 2.6.19 so we can assume it's always there.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure            | 21 ++-------------------
 linux-user/syscall.c | 18 ++++++++++--------
 2 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/configure b/configure
index b5aab72..93f8739 100755
--- a/configure
+++ b/configure
@@ -3798,8 +3798,8 @@ if compile_prog "" "" ; then
   epoll=yes
 fi
 
-# epoll_create1 and epoll_pwait are later additions
-# so we must check separately for their presence
+# epoll_create1 is a later addition
+# so we must check separately for its presence
 epoll_create1=no
 cat > $TMPC << EOF
 #include <sys/epoll.h>
@@ -3821,20 +3821,6 @@ if compile_prog "" "" ; then
   epoll_create1=yes
 fi
 
-epoll_pwait=no
-cat > $TMPC << EOF
-#include <sys/epoll.h>
-
-int main(void)
-{
-    epoll_pwait(0, 0, 0, 0, 0);
-    return 0;
-}
-EOF
-if compile_prog "" "" ; then
-  epoll_pwait=yes
-fi
-
 # check for sendfile support
 sendfile=no
 cat > $TMPC << EOF
@@ -5114,9 +5100,6 @@ fi
 if test "$epoll_create1" = "yes" ; then
   echo "CONFIG_EPOLL_CREATE1=y" >> $config_host_mak
 fi
-if test "$epoll_pwait" = "yes" ; then
-  echo "CONFIG_EPOLL_PWAIT=y" >> $config_host_mak
-fi
 if test "$sendfile" = "yes" ; then
   echo "CONFIG_SENDFILE=y" >> $config_host_mak
 fi
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f1cf809..d40d1a7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -686,6 +686,9 @@ safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds, \
 safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
               struct timespec *, tsp, const sigset_t *, sigmask,
               size_t, sigsetsize)
+safe_syscall6(int, epoll_pwait, int, epfd, struct epoll_event *, events,
+              int, maxevents, int, timeout, const sigset_t *, sigmask,
+              size_t, sigsetsize)
 safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \
               const struct timespec *,timeout,int *,uaddr2,int,val3)
 safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
@@ -10194,14 +10197,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     }
 #endif
 
-#if defined(TARGET_NR_epoll_pwait) && defined(CONFIG_EPOLL_PWAIT)
-#define IMPLEMENT_EPOLL_PWAIT
-#endif
-#if defined(TARGET_NR_epoll_wait) || defined(IMPLEMENT_EPOLL_PWAIT)
+#if defined(TARGET_NR_epoll_wait) || defined(TARGET_NR_epoll_pwait)
 #if defined(TARGET_NR_epoll_wait)
     case TARGET_NR_epoll_wait:
 #endif
-#if defined(IMPLEMENT_EPOLL_PWAIT)
+#if defined(TARGET_NR_epoll_pwait)
     case TARGET_NR_epoll_pwait:
 #endif
     {
@@ -10220,7 +10220,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         ep = alloca(maxevents * sizeof(struct epoll_event));
 
         switch (num) {
-#if defined(IMPLEMENT_EPOLL_PWAIT)
+#if defined(TARGET_NR_epoll_pwait)
         case TARGET_NR_epoll_pwait:
         {
             target_sigset_t *target_set;
@@ -10239,13 +10239,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
                 set = NULL;
             }
 
-            ret = get_errno(epoll_pwait(epfd, ep, maxevents, timeout, set));
+            ret = get_errno(safe_epoll_pwait(epfd, ep, maxevents, timeout,
+                                             set, SIGSET_T_SIZE));
             break;
         }
 #endif
 #if defined(TARGET_NR_epoll_wait)
         case TARGET_NR_epoll_wait:
-            ret = get_errno(epoll_wait(epfd, ep, maxevents, timeout));
+            ret = get_errno(safe_epoll_pwait(epfd, ep, maxevents, timeout,
+                                             NULL, 0));
             break;
 #endif
         default:
-- 
1.9.1

[Qemu-devel] [PATCH 11/18] linux-user: Use safe_syscall wrapper for semop

[Peter Maydell]

Use the safe_syscall wrapper for the semop syscall or IPC operation.
(We implement via the semtimedop syscall to make it easier to
implement the guest semtimedop syscall later.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d40d1a7..8682ab0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -719,6 +719,8 @@ safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)
 safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
               long, msgtype, int, flags)
+safe_syscall4(int, semtimedop, int, semid, struct sembuf *, tsops,
+              unsigned, nsops, const struct timespec *, timeout)
 #else
 /* This host kernel architecture uses a single ipc syscall; fake up
  * wrappers for the sub-operations to hide this implementation detail.
@@ -727,6 +729,7 @@ safe_syscall5(int, msgrcv, int, msgid, void *, msgp, size_t, sz,
  * sys/ipc.h ones. So we just define them here, and rely on them being
  * the same for all host architectures.
  */
+#define Q_SEMTIMEDOP 4
 #define Q_MSGSND 11
 #define Q_MSGRCV 12
 #define Q_IPCCALL(VERSION, OP) ((VERSION) << 16 | (OP))
@@ -741,6 +744,12 @@ static int safe_msgrcv(int msgid, void *msgp, size_t sz, long type, int flags)
 {
     return safe_ipc(Q_IPCCALL(1, Q_MSGRCV), msgid, sz, flags, msgp, type);
 }
+static int safe_semtimedop(int semid, struct sembuf *tsops, unsigned nsops,
+                           const struct timespec *timeout)
+{
+    return safe_ipc(Q_IPCCALL(0, Q_SEMTIMEDOP), semid, nsops, 0, tsops,
+                    (long)timeout);
+}
 #endif
 #if defined(TARGET_NR_mq_open) && defined(__NR_mq_open)
 safe_syscall5(int, mq_timedsend, int, mqdes, const char *, msg_ptr,
@@ -3039,7 +3048,7 @@ static inline abi_long do_semop(int semid, abi_long ptr, unsigned nsops)
     if (target_to_host_sembuf(sops, ptr, nsops))
         return -TARGET_EFAULT;
 
-    return get_errno(semop(semid, sops, nsops));
+    return get_errno(safe_semtimedop(semid, sops, nsops, NULL));
 }
 
 struct target_msqid_ds
-- 
1.9.1

[Qemu-devel] [PATCH 12/18] linux-user: Use safe_syscall wrapper for accept and accept4 syscalls

[Peter Maydell]

Use the safe_syscall wrapper for the accept and accept4 syscalls.
accept4 has been in the kernel since 2.6.28 so we can assume it
is always present.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8682ab0..aac2bbd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -708,6 +708,8 @@ safe_syscall3(ssize_t, recvmsg, int, fd, struct msghdr *, msg, int, flags)
 safe_syscall2(int, flock, int, fd, int, operation)
 safe_syscall4(int, rt_sigtimedwait, const sigset_t *, these, siginfo_t *, uinfo,
               const struct timespec *, uts, size_t, sigsetsize)
+safe_syscall4(int, accept4, int, fd, struct sockaddr *, addr, socklen_t *, len,
+              int, flags)
 safe_syscall2(int, nanosleep, const struct timespec *, req,
               struct timespec *, rem)
 #ifdef TARGET_NR_clock_nanosleep
@@ -2427,19 +2429,6 @@ static abi_long do_sendrecvmmsg(int fd, abi_ulong target_msgvec,
     return ret;
 }
 
-/* If we don't have a system accept4() then just call accept.
- * The callsites to do_accept4() will ensure that they don't
- * pass a non-zero flags argument in this config.
- */
-#ifndef CONFIG_ACCEPT4
-static inline int accept4(int sockfd, struct sockaddr *addr,
-                          socklen_t *addrlen, int flags)
-{
-    assert(flags == 0);
-    return accept(sockfd, addr, addrlen);
-}
-#endif
-
 /* do_accept4() Must return target values and target errnos. */
 static abi_long do_accept4(int fd, abi_ulong target_addr,
                            abi_ulong target_addrlen_addr, int flags)
@@ -2452,7 +2441,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
     host_flags = target_to_host_bitmask(flags, fcntl_flags_tbl);
 
     if (target_addr == 0) {
-        return get_errno(accept4(fd, NULL, NULL, host_flags));
+        return get_errno(safe_accept4(fd, NULL, NULL, host_flags));
     }
 
     /* linux returns EINVAL if addrlen pointer is invalid */
@@ -2468,7 +2457,7 @@ static abi_long do_accept4(int fd, abi_ulong target_addr,
 
     addr = alloca(addrlen);
 
-    ret = get_errno(accept4(fd, addr, &addrlen, host_flags));
+    ret = get_errno(safe_accept4(fd, addr, &addrlen, host_flags));
     if (!is_error(ret)) {
         host_to_target_sockaddr(target_addr, addr, addrlen);
         if (put_user_u32(addrlen, target_addrlen_addr))
@@ -7693,11 +7682,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_accept4
     case TARGET_NR_accept4:
-#ifdef CONFIG_ACCEPT4
         ret = do_accept4(arg1, arg2, arg3, arg4);
-#else
-        goto unimplemented;
-#endif
         break;
 #endif
 #ifdef TARGET_NR_bind
-- 
1.9.1

[Qemu-devel] [PATCH 13/18] linux-user: Use safe_syscall wrapper for ioctl

[Peter Maydell]

Use the safe_syscall wrapper to implement the ioctl syscall.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index aac2bbd..4cf67c8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -759,6 +759,11 @@ safe_syscall5(int, mq_timedsend, int, mqdes, const char *, msg_ptr,
 safe_syscall5(int, mq_timedreceive, int, mqdes, char *, msg_ptr,
               size_t, len, unsigned *, prio, const struct timespec *, timeout)
 #endif
+/* We do ioctl like this rather than via safe_syscall3 to preserve the
+ * "third argument might be integer or pointer or not present" behaviour of
+ * the libc function.
+ */
+#define safe_ioctl(...) safe_syscall(__NR_ioctl, __VA_ARGS__)
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -3636,7 +3641,7 @@ static abi_long do_ioctl_fs_ioc_fiemap(const IOCTLEntry *ie, uint8_t *buf_temp,
         memcpy(fm, buf_temp, sizeof(struct fiemap));
         free_fm = 1;
     }
-    ret = get_errno(ioctl(fd, ie->host_cmd, fm));
+    ret = get_errno(safe_ioctl(fd, ie->host_cmd, fm));
     if (!is_error(ret)) {
         target_size_out = target_size_in;
         /* An extent_count of 0 means we were only counting the extents
@@ -3726,7 +3731,7 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
     host_ifconf->ifc_len = host_ifc_len;
     host_ifconf->ifc_buf = host_ifc_buf;
 
-    ret = get_errno(ioctl(fd, ie->host_cmd, host_ifconf));
+    ret = get_errno(safe_ioctl(fd, ie->host_cmd, host_ifconf));
     if (!is_error(ret)) {
 	/* convert host ifc_len to target ifc_len */
 
@@ -3855,7 +3860,7 @@ static abi_long do_ioctl_dm(const IOCTLEntry *ie, uint8_t *buf_temp, int fd,
     }
     unlock_user(argptr, guest_data, 0);
 
-    ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+    ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
     if (!is_error(ret)) {
         guest_data = arg + host_dm->data_start;
         guest_data_size = host_dm->data_size - host_dm->data_start;
@@ -4036,7 +4041,7 @@ static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t *buf_temp, int fd,
 
     /* Swizzle the data pointer to our local copy and call! */
     host_blkpg->data = &host_part;
-    ret = get_errno(ioctl(fd, ie->host_cmd, host_blkpg));
+    ret = get_errno(safe_ioctl(fd, ie->host_cmd, host_blkpg));
 
 out:
     return ret;
@@ -4097,7 +4102,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
     }
     unlock_user(argptr, arg, 0);
 
-    ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+    ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
     if (*host_rt_dev_ptr != 0) {
         unlock_user((void *)*host_rt_dev_ptr,
                     *target_rt_dev_ptr, 0);
@@ -4109,7 +4114,7 @@ static abi_long do_ioctl_kdsigaccept(const IOCTLEntry *ie, uint8_t *buf_temp,
                                      int fd, int cmd, abi_long arg)
 {
     int sig = target_to_host_signal(arg);
-    return get_errno(ioctl(fd, ie->host_cmd, sig));
+    return get_errno(safe_ioctl(fd, ie->host_cmd, sig));
 }
 
 static IOCTLEntry ioctl_entries[] = {
@@ -4153,18 +4158,18 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
     switch(arg_type[0]) {
     case TYPE_NULL:
         /* no argument */
-        ret = get_errno(ioctl(fd, ie->host_cmd));
+        ret = get_errno(safe_ioctl(fd, ie->host_cmd));
         break;
     case TYPE_PTRVOID:
     case TYPE_INT:
-        ret = get_errno(ioctl(fd, ie->host_cmd, arg));
+        ret = get_errno(safe_ioctl(fd, ie->host_cmd, arg));
         break;
     case TYPE_PTR:
         arg_type++;
         target_size = thunk_type_size(arg_type, 0);
         switch(ie->access) {
         case IOC_R:
-            ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+            ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
             if (!is_error(ret)) {
                 argptr = lock_user(VERIFY_WRITE, arg, target_size, 0);
                 if (!argptr)
@@ -4179,7 +4184,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
                 return -TARGET_EFAULT;
             thunk_convert(buf_temp, argptr, arg_type, THUNK_HOST);
             unlock_user(argptr, arg, 0);
-            ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+            ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
             break;
         default:
         case IOC_RW:
@@ -4188,7 +4193,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
                 return -TARGET_EFAULT;
             thunk_convert(buf_temp, argptr, arg_type, THUNK_HOST);
             unlock_user(argptr, arg, 0);
-            ret = get_errno(ioctl(fd, ie->host_cmd, buf_temp));
+            ret = get_errno(safe_ioctl(fd, ie->host_cmd, buf_temp));
             if (!is_error(ret)) {
                 argptr = lock_user(VERIFY_WRITE, arg, target_size, 0);
                 if (!argptr)
-- 
1.9.1

[Qemu-devel] [PATCH 14/18] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()

[Peter Maydell]

Use the __get_user() and __put_user() to handle reading and writing the
guest structures in do_ioctl(). This has two benefits:
 * avoids possible errors due to misaligned guest pointers
 * correctly sign extends signed fields (like l_start in struct flock)
   which might be different sizes between guest and host

To do this we abstract out into copy_from/to_user functions. We
also standardize on always using host flock64 and the F_GETLK64
etc flock commands, as this means we always have 64 bit offsets
whether the host is 64-bit or 32-bit and we don't need to support
conversion to both host struct flock and struct flock64.

In passing we fix errors in converting l_type from the host to
the target (where we were doing a byteswap of the host value
before trying to do the convert-bitmasks operation rather than
otherwise, and inexplicably shifting left by 1).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 280 +++++++++++++++++++++++++++++----------------------
 1 file changed, 157 insertions(+), 123 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 4cf67c8..f3a487e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4894,11 +4894,11 @@ static int target_to_host_fcntl_cmd(int cmd)
 	case TARGET_F_SETFL:
             return cmd;
         case TARGET_F_GETLK:
-	    return F_GETLK;
-	case TARGET_F_SETLK:
-	    return F_SETLK;
-	case TARGET_F_SETLKW:
-	    return F_SETLKW;
+            return F_GETLK64;
+        case TARGET_F_SETLK:
+            return F_SETLK64;
+        case TARGET_F_SETLKW:
+            return F_SETLKW64;
 	case TARGET_F_GETOWN:
 	    return F_GETOWN;
 	case TARGET_F_SETOWN:
@@ -4949,12 +4949,134 @@ static const bitmask_transtbl flock_tbl[] = {
     { 0, 0, 0, 0 }
 };
 
-static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
+static inline abi_long copy_from_user_flock(struct flock64 *fl,
+                                            abi_ulong target_flock_addr)
 {
-    struct flock fl;
     struct target_flock *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+        return -TARGET_EFAULT;
+    }
+
+    __get_user(l_type, &target_fl->l_type);
+    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+    __get_user(fl->l_whence, &target_fl->l_whence);
+    __get_user(fl->l_start, &target_fl->l_start);
+    __get_user(fl->l_len, &target_fl->l_len);
+    __get_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 0);
+    return 0;
+}
+
+static inline abi_long copy_to_user_flock(abi_ulong target_flock_addr,
+                                          const struct flock64 *fl)
+{
+    struct target_flock *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+        return -TARGET_EFAULT;
+    }
+
+    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+    __put_user(l_type, &target_fl->l_type);
+    __put_user(fl->l_whence, &target_fl->l_whence);
+    __put_user(fl->l_start, &target_fl->l_start);
+    __put_user(fl->l_len, &target_fl->l_len);
+    __put_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 1);
+    return 0;
+}
+
+typedef abi_long from_flock64_fn(struct flock64 *fl, abi_ulong target_addr);
+typedef abi_long to_flock64_fn(abi_ulong target_addr, const struct flock64 *fl);
+
+#ifdef TARGET_ARM
+static inline abi_long copy_from_user_eabi_flock64(struct flock64 *fl,
+                                                   abi_ulong target_flock_addr)
+{
+    struct target_eabi_flock64 *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+        return -TARGET_EFAULT;
+    }
+
+    __get_user(l_type, &target_fl->l_type);
+    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+    __get_user(fl->l_whence, &target_fl->l_whence);
+    __get_user(fl->l_start, &target_fl->l_start);
+    __get_user(fl->l_len, &target_fl->l_len);
+    __get_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 0);
+    return 0;
+}
+
+static inline abi_long copy_to_user_eabi_flock64(abi_ulong target_flock_addr,
+                                                 const struct flock64 *fl)
+{
+    struct target_eabi_flock64 *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+        return -TARGET_EFAULT;
+    }
+
+    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+    __put_user(l_type, &target_fl->l_type);
+    __put_user(fl->l_whence, &target_fl->l_whence);
+    __put_user(fl->l_start, &target_fl->l_start);
+    __put_user(fl->l_len, &target_fl->l_len);
+    __put_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 1);
+    return 0;
+}
+#endif
+
+static inline abi_long copy_from_user_flock64(struct flock64 *fl,
+                                              abi_ulong target_flock_addr)
+{
+    struct target_flock64 *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
+        return -TARGET_EFAULT;
+    }
+
+    __get_user(l_type, &target_fl->l_type);
+    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
+    __get_user(fl->l_whence, &target_fl->l_whence);
+    __get_user(fl->l_start, &target_fl->l_start);
+    __get_user(fl->l_len, &target_fl->l_len);
+    __get_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 0);
+    return 0;
+}
+
+static inline abi_long copy_to_user_flock64(abi_ulong target_flock_addr,
+                                            const struct flock64 *fl)
+{
+    struct target_flock64 *target_fl;
+    short l_type;
+
+    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
+        return -TARGET_EFAULT;
+    }
+
+    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
+    __put_user(l_type, &target_fl->l_type);
+    __put_user(fl->l_whence, &target_fl->l_whence);
+    __put_user(fl->l_start, &target_fl->l_start);
+    __put_user(fl->l_len, &target_fl->l_len);
+    __put_user(fl->l_pid, &target_fl->l_pid);
+    unlock_user_struct(target_fl, target_flock_addr, 1);
+    return 0;
+}
+
+static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
+{
     struct flock64 fl64;
-    struct target_flock64 *target_fl64;
 #ifdef F_GETOWN_EX
     struct f_owner_ex fox;
     struct target_f_owner_ex *target_fox;
@@ -4967,77 +5089,41 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
 
     switch(cmd) {
     case TARGET_F_GETLK:
-        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
+        if (copy_from_user_flock(&fl64, arg)) {
             return -TARGET_EFAULT;
-        fl.l_type =
-                  target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
-        fl.l_whence = tswap16(target_fl->l_whence);
-        fl.l_start = tswapal(target_fl->l_start);
-        fl.l_len = tswapal(target_fl->l_len);
-        fl.l_pid = tswap32(target_fl->l_pid);
-        unlock_user_struct(target_fl, arg, 0);
-        ret = get_errno(fcntl(fd, host_cmd, &fl));
+        }
+        ret = get_errno(fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
-            if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
+            if (copy_to_user_flock(arg, &fl64)) {
                 return -TARGET_EFAULT;
-            target_fl->l_type =
-                          host_to_target_bitmask(tswap16(fl.l_type), flock_tbl);
-            target_fl->l_whence = tswap16(fl.l_whence);
-            target_fl->l_start = tswapal(fl.l_start);
-            target_fl->l_len = tswapal(fl.l_len);
-            target_fl->l_pid = tswap32(fl.l_pid);
-            unlock_user_struct(target_fl, arg, 1);
+            }
         }
         break;
 
     case TARGET_F_SETLK:
     case TARGET_F_SETLKW:
-        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
+        if (copy_from_user_flock(&fl64, arg)) {
             return -TARGET_EFAULT;
-        fl.l_type =
-                  target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
-        fl.l_whence = tswap16(target_fl->l_whence);
-        fl.l_start = tswapal(target_fl->l_start);
-        fl.l_len = tswapal(target_fl->l_len);
-        fl.l_pid = tswap32(target_fl->l_pid);
-        unlock_user_struct(target_fl, arg, 0);
-        ret = get_errno(fcntl(fd, host_cmd, &fl));
+        }
+        ret = get_errno(fcntl(fd, host_cmd, &fl64));
         break;
 
     case TARGET_F_GETLK64:
-        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
+        if (copy_from_user_flock64(&fl64, arg)) {
             return -TARGET_EFAULT;
-        fl64.l_type =
-           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
-        fl64.l_whence = tswap16(target_fl64->l_whence);
-        fl64.l_start = tswap64(target_fl64->l_start);
-        fl64.l_len = tswap64(target_fl64->l_len);
-        fl64.l_pid = tswap32(target_fl64->l_pid);
-        unlock_user_struct(target_fl64, arg, 0);
+        }
         ret = get_errno(fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
-            if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
+            if (copy_to_user_flock64(arg, &fl64)) {
                 return -TARGET_EFAULT;
-            target_fl64->l_type =
-                   host_to_target_bitmask(tswap16(fl64.l_type), flock_tbl) >> 1;
-            target_fl64->l_whence = tswap16(fl64.l_whence);
-            target_fl64->l_start = tswap64(fl64.l_start);
-            target_fl64->l_len = tswap64(fl64.l_len);
-            target_fl64->l_pid = tswap32(fl64.l_pid);
-            unlock_user_struct(target_fl64, arg, 1);
+            }
         }
         break;
     case TARGET_F_SETLK64:
     case TARGET_F_SETLKW64:
-        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
+        if (copy_from_user_flock64(&fl64, arg)) {
             return -TARGET_EFAULT;
-        fl64.l_type =
-           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
-        fl64.l_whence = tswap16(target_fl64->l_whence);
-        fl64.l_start = tswap64(target_fl64->l_start);
-        fl64.l_len = tswap64(target_fl64->l_len);
-        fl64.l_pid = tswap32(target_fl64->l_pid);
-        unlock_user_struct(target_fl64, arg, 0);
+        }
         ret = get_errno(fcntl(fd, host_cmd, &fl64));
         break;
 
@@ -9485,9 +9571,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     {
 	int cmd;
 	struct flock64 fl;
-	struct target_flock64 *target_fl;
+        from_flock64_fn *copyfrom = copy_from_user_flock64;
+        to_flock64_fn *copyto = copy_to_user_flock64;
+
 #ifdef TARGET_ARM
-	struct target_eabi_flock64 *target_efl;
+        if (((CPUARMState *)cpu_env)->eabi) {
+            copyfrom = copy_from_user_eabi_flock64;
+            copyto = copy_to_user_eabi_flock64;
+        }
 #endif
 
 	cmd = target_to_host_fcntl_cmd(arg2);
@@ -9498,78 +9589,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
         switch(arg2) {
         case TARGET_F_GETLK64:
-#ifdef TARGET_ARM
-            if (((CPUARMState *)cpu_env)->eabi) {
-                if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
-                    goto efault;
-                fl.l_type = tswap16(target_efl->l_type);
-                fl.l_whence = tswap16(target_efl->l_whence);
-                fl.l_start = tswap64(target_efl->l_start);
-                fl.l_len = tswap64(target_efl->l_len);
-                fl.l_pid = tswap32(target_efl->l_pid);
-                unlock_user_struct(target_efl, arg3, 0);
-            } else
-#endif
-            {
-                if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
-                    goto efault;
-                fl.l_type = tswap16(target_fl->l_type);
-                fl.l_whence = tswap16(target_fl->l_whence);
-                fl.l_start = tswap64(target_fl->l_start);
-                fl.l_len = tswap64(target_fl->l_len);
-                fl.l_pid = tswap32(target_fl->l_pid);
-                unlock_user_struct(target_fl, arg3, 0);
+            if (copyfrom(&fl, arg3)) {
+                goto efault;
             }
             ret = get_errno(fcntl(arg1, cmd, &fl));
 	    if (ret == 0) {
-#ifdef TARGET_ARM
-                if (((CPUARMState *)cpu_env)->eabi) {
-                    if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
-                        goto efault;
-                    target_efl->l_type = tswap16(fl.l_type);
-                    target_efl->l_whence = tswap16(fl.l_whence);
-                    target_efl->l_start = tswap64(fl.l_start);
-                    target_efl->l_len = tswap64(fl.l_len);
-                    target_efl->l_pid = tswap32(fl.l_pid);
-                    unlock_user_struct(target_efl, arg3, 1);
-                } else
-#endif
-                {
-                    if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
-                        goto efault;
-                    target_fl->l_type = tswap16(fl.l_type);
-                    target_fl->l_whence = tswap16(fl.l_whence);
-                    target_fl->l_start = tswap64(fl.l_start);
-                    target_fl->l_len = tswap64(fl.l_len);
-                    target_fl->l_pid = tswap32(fl.l_pid);
-                    unlock_user_struct(target_fl, arg3, 1);
+                if (copyto(arg3, &fl)) {
+                    goto efault;
                 }
 	    }
 	    break;
 
         case TARGET_F_SETLK64:
         case TARGET_F_SETLKW64:
-#ifdef TARGET_ARM
-            if (((CPUARMState *)cpu_env)->eabi) {
-                if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
-                    goto efault;
-                fl.l_type = tswap16(target_efl->l_type);
-                fl.l_whence = tswap16(target_efl->l_whence);
-                fl.l_start = tswap64(target_efl->l_start);
-                fl.l_len = tswap64(target_efl->l_len);
-                fl.l_pid = tswap32(target_efl->l_pid);
-                unlock_user_struct(target_efl, arg3, 0);
-            } else
-#endif
-            {
-                if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
-                    goto efault;
-                fl.l_type = tswap16(target_fl->l_type);
-                fl.l_whence = tswap16(target_fl->l_whence);
-                fl.l_start = tswap64(target_fl->l_start);
-                fl.l_len = tswap64(target_fl->l_len);
-                fl.l_pid = tswap32(target_fl->l_pid);
-                unlock_user_struct(target_fl, arg3, 0);
+            if (copyfrom(&fl, arg3)) {
+                goto efault;
             }
             ret = get_errno(fcntl(arg1, cmd, &fl));
 	    break;
-- 
1.9.1

[Qemu-devel] [PATCH 15/18] linux-user: Correct signedness of target_flock l_start and l_len fields

[Peter Maydell]

The l_start and l_len fields in the various target_flock structures are
supposed to be '__kernel_off_t' or '__kernel_loff_t', which means they
should be signed, not unsigned. Correcting the structure definitions means
that __get_user() and __put_user() will correctly sign extend them if
the guest is using 32 bit offsets and the host is using 64 bit offsets.

This fixes failures in the LTP 'fcntl14' tests where it checks that
negative seek offsets work correctly.

We reindent the structures to drop hard tabs since we're touching 40%
of the fields anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall_defs.h | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 124754f..8a801e0 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2289,34 +2289,34 @@ struct target_statfs64 {
 #endif
 
 struct target_flock {
-	short l_type;
-	short l_whence;
-	abi_ulong l_start;
-	abi_ulong l_len;
-	int l_pid;
+    short l_type;
+    short l_whence;
+    abi_long l_start;
+    abi_long l_len;
+    int l_pid;
 };
 
 struct target_flock64 {
-	short  l_type;
-	short  l_whence;
+    short  l_type;
+    short  l_whence;
 #if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
     || defined(TARGET_SPARC) || defined(TARGET_HPPA) \
     || defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX)
-        int __pad;
+    int __pad;
 #endif
-	unsigned long long l_start;
-	unsigned long long l_len;
-	int  l_pid;
+    long long l_start;
+    long long l_len;
+    int  l_pid;
 } QEMU_PACKED;
 
 #ifdef TARGET_ARM
 struct target_eabi_flock64 {
-	short  l_type;
-	short  l_whence;
-        int __pad;
-	unsigned long long l_start;
-	unsigned long long l_len;
-	int  l_pid;
+    short  l_type;
+    short  l_whence;
+    int __pad;
+    long long l_start;
+    long long l_len;
+    int  l_pid;
 } QEMU_PACKED;
 #endif
 
-- 
1.9.1

[Qemu-devel] [PATCH 16/18] linux-user: Use safe_syscall wrapper for fcntl

[Peter Maydell]

Use the safe_syscall wrapper for fcntl. This is straightforward now
that we always use 'struct fcntl64' on the host, as we don't need
to select whether to call the host's fcntl64 or fcntl syscall
(a detail that the libc previously hid for us).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f3a487e..249d246 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -764,6 +764,16 @@ safe_syscall5(int, mq_timedreceive, int, mqdes, char *, msg_ptr,
  * the libc function.
  */
 #define safe_ioctl(...) safe_syscall(__NR_ioctl, __VA_ARGS__)
+/* Similarly for fcntl. Note that callers must always:
+ *  pass the F_GETLK64 etc constants rather than the unsuffixed F_GETLK
+ *  use the flock64 struct rather than unsuffixed flock
+ * This will then work and use a 64-bit offset for both 32-bit and 64-bit hosts.
+ */
+#ifdef __NR_fcntl64
+#define safe_fcntl(...) safe_syscall(__NR_fcntl64, __VA_ARGS__)
+#else
+#define safe_fcntl(...) safe_syscall(__NR_fcntl, __VA_ARGS__)
+#endif
 
 static inline int host_to_target_sock_type(int host_type)
 {
@@ -5092,7 +5102,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (copy_from_user_flock(&fl64, arg)) {
             return -TARGET_EFAULT;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
             if (copy_to_user_flock(arg, &fl64)) {
                 return -TARGET_EFAULT;
@@ -5105,14 +5115,14 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (copy_from_user_flock(&fl64, arg)) {
             return -TARGET_EFAULT;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         break;
 
     case TARGET_F_GETLK64:
         if (copy_from_user_flock64(&fl64, arg)) {
             return -TARGET_EFAULT;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
             if (copy_to_user_flock64(arg, &fl64)) {
                 return -TARGET_EFAULT;
@@ -5124,23 +5134,25 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (copy_from_user_flock64(&fl64, arg)) {
             return -TARGET_EFAULT;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         break;
 
     case TARGET_F_GETFL:
-        ret = get_errno(fcntl(fd, host_cmd, arg));
+        ret = get_errno(safe_fcntl(fd, host_cmd, arg));
         if (ret >= 0) {
             ret = host_to_target_bitmask(ret, fcntl_flags_tbl);
         }
         break;
 
     case TARGET_F_SETFL:
-        ret = get_errno(fcntl(fd, host_cmd, target_to_host_bitmask(arg, fcntl_flags_tbl)));
+        ret = get_errno(safe_fcntl(fd, host_cmd,
+                                   target_to_host_bitmask(arg,
+                                                          fcntl_flags_tbl)));
         break;
 
 #ifdef F_GETOWN_EX
     case TARGET_F_GETOWN_EX:
-        ret = get_errno(fcntl(fd, host_cmd, &fox));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
         if (ret >= 0) {
             if (!lock_user_struct(VERIFY_WRITE, target_fox, arg, 0))
                 return -TARGET_EFAULT;
@@ -5158,7 +5170,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         fox.type = tswap32(target_fox->type);
         fox.pid = tswap32(target_fox->pid);
         unlock_user_struct(target_fox, arg, 0);
-        ret = get_errno(fcntl(fd, host_cmd, &fox));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fox));
         break;
 #endif
 
@@ -5168,11 +5180,11 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
     case TARGET_F_GETSIG:
     case TARGET_F_SETLEASE:
     case TARGET_F_GETLEASE:
-        ret = get_errno(fcntl(fd, host_cmd, arg));
+        ret = get_errno(safe_fcntl(fd, host_cmd, arg));
         break;
 
     default:
-        ret = get_errno(fcntl(fd, cmd, arg));
+        ret = get_errno(safe_fcntl(fd, cmd, arg));
         break;
     }
     return ret;
@@ -9605,7 +9617,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (copyfrom(&fl, arg3)) {
                 goto efault;
             }
-            ret = get_errno(fcntl(arg1, cmd, &fl));
+            ret = get_errno(safe_fcntl(arg1, cmd, &fl));
 	    break;
         default:
             ret = do_fcntl(arg1, arg2, arg3);
-- 
1.9.1

[Qemu-devel] [PATCH 17/18] linux-user: Make target_strerror() return 'const char *'

[Peter Maydell]

Make target_strerror() return 'const char *' rather than just 'char *';
this will allow us to return constant strings from it for some special
cases.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/qemu.h    | 2 +-
 linux-user/strace.c  | 4 ++--
 linux-user/syscall.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 6bd7b32..56f29c3 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -195,7 +195,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 extern THREAD CPUState *thread_cpu;
 void cpu_loop(CPUArchState *env);
-char *target_strerror(int err);
+const char *target_strerror(int err);
 int get_osversion(void);
 void init_qemu_uname_release(void);
 void fork_start(void);
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 0810c85..c5980a1 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -281,7 +281,7 @@ print_ipc(const struct syscallname *name,
 static void
 print_syscall_ret_addr(const struct syscallname *name, abi_long ret)
 {
-    char *errstr = NULL;
+    const char *errstr = NULL;
 
     if (ret < 0) {
         errstr = target_strerror(-ret);
@@ -1594,7 +1594,7 @@ void
 print_syscall_ret(int num, abi_long ret)
 {
     int i;
-    char *errstr = NULL;
+    const char *errstr = NULL;
 
     for(i=0;i<nsyscalls;i++)
         if( scnames[i].nr == num ) {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 249d246..bcee02d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -617,7 +617,7 @@ static inline int is_error(abi_long ret)
     return (abi_ulong)ret >= (abi_ulong)(-4096);
 }
 
-char *target_strerror(int err)
+const char *target_strerror(int err)
 {
     if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
         return NULL;
-- 
1.9.1

[Qemu-devel] [PATCH 18/18] linux-user: Special-case ERESTARTSYS in target_strerror()

[Peter Maydell]

Since TARGET_ERESTARTSYS and TARGET_ESIGRETURN are internal-to-QEMU
error numbers, handle them specially in target_strerror(), to avoid
confusing strace output like:

9521 rt_sigreturn(14,8,274886297808,8,0,268435456) = -1 errno=513 (Unknown error 513)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index bcee02d..782d475 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -619,6 +619,13 @@ static inline int is_error(abi_long ret)
 
 const char *target_strerror(int err)
 {
+    if (err == TARGET_ERESTARTSYS) {
+        return "To be restarted";
+    }
+    if (err == TARGET_QEMU_ESIGRETURN) {
+        return "Successful exit from sigreturn";
+    }
+
     if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
         return NULL;
     }
-- 
1.9.1

Re: [Qemu-devel] [PATCH 18/18] linux-user: Special-case ERESTARTSYS in target_strerror()

[Laurent Vivier]

Le 06/06/2016 à 20:58, Peter Maydell a écrit :
> Since TARGET_ERESTARTSYS and TARGET_ESIGRETURN are internal-to-QEMU
> error numbers, handle them specially in target_strerror(), to avoid
> confusing strace output like:
> 
> 9521 rt_sigreturn(14,8,274886297808,8,0,268435456) = -1 errno=513 (Unknown error 513)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index bcee02d..782d475 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -619,6 +619,13 @@ static inline int is_error(abi_long ret)
>  
>  const char *target_strerror(int err)
>  {
> +    if (err == TARGET_ERESTARTSYS) {
> +        return "To be restarted";
> +    }
> +    if (err == TARGET_QEMU_ESIGRETURN) {
> +        return "Successful exit from sigreturn";
> +    }
> +
>      if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
>          return NULL;
>      }

This is not the aim of this patch, but target_to_host_errno() has now
these checks, perhaps we can remove this while we are here...

Laurent

Re: [Qemu-devel] [PATCH 17/18] linux-user: Make target_strerror() return 'const char *'

[Laurent Vivier]

Le 06/06/2016 à 20:58, Peter Maydell a écrit :
> Make target_strerror() return 'const char *' rather than just 'char *';
> this will allow us to return constant strings from it for some special
> cases.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  linux-user/qemu.h    | 2 +-
>  linux-user/strace.c  | 4 ++--
>  linux-user/syscall.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 6bd7b32..56f29c3 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -195,7 +195,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  extern THREAD CPUState *thread_cpu;
>  void cpu_loop(CPUArchState *env);
> -char *target_strerror(int err);
> +const char *target_strerror(int err);
>  int get_osversion(void);
>  void init_qemu_uname_release(void);
>  void fork_start(void);
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 0810c85..c5980a1 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -281,7 +281,7 @@ print_ipc(const struct syscallname *name,
>  static void
>  print_syscall_ret_addr(const struct syscallname *name, abi_long ret)
>  {
> -    char *errstr = NULL;
> +    const char *errstr = NULL;
>  
>      if (ret < 0) {
>          errstr = target_strerror(-ret);
> @@ -1594,7 +1594,7 @@ void
>  print_syscall_ret(int num, abi_long ret)
>  {
>      int i;
> -    char *errstr = NULL;
> +    const char *errstr = NULL;
>  
>      for(i=0;i<nsyscalls;i++)
>          if( scnames[i].nr == num ) {
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 249d246..bcee02d 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -617,7 +617,7 @@ static inline int is_error(abi_long ret)
>      return (abi_ulong)ret >= (abi_ulong)(-4096);
>  }
>  
> -char *target_strerror(int err)
> +const char *target_strerror(int err)
>  {
>      if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
>          return NULL;
> 

Re: [Qemu-devel] [PATCH 15/18] linux-user: Correct signedness of target_flock l_start and l_len fields

[Laurent Vivier]

Le 06/06/2016 à 20:58, Peter Maydell a écrit :
> The l_start and l_len fields in the various target_flock structures are
> supposed to be '__kernel_off_t' or '__kernel_loff_t', which means they
> should be signed, not unsigned. Correcting the structure definitions means
> that __get_user() and __put_user() will correctly sign extend them if
> the guest is using 32 bit offsets and the host is using 64 bit offsets.
> 
> This fixes failures in the LTP 'fcntl14' tests where it checks that
> negative seek offsets work correctly.
> 
> We reindent the structures to drop hard tabs since we're touching 40%
> of the fields anyway.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall_defs.h | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 124754f..8a801e0 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2289,34 +2289,34 @@ struct target_statfs64 {
>  #endif
>  
>  struct target_flock {
> -	short l_type;
> -	short l_whence;
> -	abi_ulong l_start;
> -	abi_ulong l_len;
> -	int l_pid;
> +    short l_type;
> +    short l_whence;
> +    abi_long l_start;
> +    abi_long l_len;
> +    int l_pid;
>  };
>  
>  struct target_flock64 {
> -	short  l_type;
> -	short  l_whence;
> +    short  l_type;
> +    short  l_whence;
>  #if defined(TARGET_PPC) || defined(TARGET_X86_64) || defined(TARGET_MIPS) \
>      || defined(TARGET_SPARC) || defined(TARGET_HPPA) \
>      || defined(TARGET_MICROBLAZE) || defined(TARGET_TILEGX)
> -        int __pad;
> +    int __pad;
>  #endif
> -	unsigned long long l_start;
> -	unsigned long long l_len;
> -	int  l_pid;
> +    long long l_start;
> +    long long l_len;

to be correct, they should be abi_llong.

> +    int  l_pid;
>  } QEMU_PACKED;
>  
>  #ifdef TARGET_ARM
>  struct target_eabi_flock64 {
> -	short  l_type;
> -	short  l_whence;
> -        int __pad;
> -	unsigned long long l_start;
> -	unsigned long long l_len;
> -	int  l_pid;
> +    short  l_type;
> +    short  l_whence;
> +    int __pad;
> +    long long l_start;
> +    long long l_len;

abi_llong

> +    int  l_pid;
>  } QEMU_PACKED;
>  #endif
>  
> 

Re: [Qemu-devel] [PATCH 14/18] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()

[Laurent Vivier]

Le 06/06/2016 à 20:58, Peter Maydell a écrit :
> Use the __get_user() and __put_user() to handle reading and writing the
> guest structures in do_ioctl(). This has two benefits:
>  * avoids possible errors due to misaligned guest pointers
>  * correctly sign extends signed fields (like l_start in struct flock)
>    which might be different sizes between guest and host
> 
> To do this we abstract out into copy_from/to_user functions. We
> also standardize on always using host flock64 and the F_GETLK64
> etc flock commands, as this means we always have 64 bit offsets
> whether the host is 64-bit or 32-bit and we don't need to support
> conversion to both host struct flock and struct flock64.
> 
> In passing we fix errors in converting l_type from the host to
> the target (where we were doing a byteswap of the host value
> before trying to do the convert-bitmasks operation rather than
> otherwise, and inexplicably shifting left by 1).

I  think the ">> 1" is coming from:

43f238d Support fcntl F_GETLK64, F_SETLK64, F_SETLKW64

to convert arm to x86, and should have been removed then in:

2ba7f73 alpha-linux-user: Translate fcntl l_type

So yes, the ">> 1" is wrong. I don't understand how it can work.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c | 280 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 157 insertions(+), 123 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 4cf67c8..f3a487e 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4894,11 +4894,11 @@ static int target_to_host_fcntl_cmd(int cmd)
>  	case TARGET_F_SETFL:
>              return cmd;
>          case TARGET_F_GETLK:
> -	    return F_GETLK;
> -	case TARGET_F_SETLK:
> -	    return F_SETLK;
> -	case TARGET_F_SETLKW:
> -	    return F_SETLKW;
> +            return F_GETLK64;
> +        case TARGET_F_SETLK:
> +            return F_SETLK64;
> +        case TARGET_F_SETLKW:
> +            return F_SETLKW64;
>  	case TARGET_F_GETOWN:
>  	    return F_GETOWN;
>  	case TARGET_F_SETOWN:

I see no reason to have this in this patch.

> @@ -4949,12 +4949,134 @@ static const bitmask_transtbl flock_tbl[] = {
>      { 0, 0, 0, 0 }
>  };
>  
> -static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> +static inline abi_long copy_from_user_flock(struct flock64 *fl,
> +                                            abi_ulong target_flock_addr)
>  {
> -    struct flock fl;
>      struct target_flock *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    __get_user(l_type, &target_fl->l_type);
> +    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> +    __get_user(fl->l_whence, &target_fl->l_whence);
> +    __get_user(fl->l_start, &target_fl->l_start);
> +    __get_user(fl->l_len, &target_fl->l_len);
> +    __get_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 0);
> +    return 0;
> +}
> +
> +static inline abi_long copy_to_user_flock(abi_ulong target_flock_addr,
> +                                          const struct flock64 *fl)
> +{
> +    struct target_flock *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> +    __put_user(l_type, &target_fl->l_type);
> +    __put_user(fl->l_whence, &target_fl->l_whence);
> +    __put_user(fl->l_start, &target_fl->l_start);
> +    __put_user(fl->l_len, &target_fl->l_len);
> +    __put_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 1);
> +    return 0;
> +}
> +
> +typedef abi_long from_flock64_fn(struct flock64 *fl, abi_ulong target_addr);
> +typedef abi_long to_flock64_fn(abi_ulong target_addr, const struct flock64 *fl);
> +
> +#ifdef TARGET_ARM
> +static inline abi_long copy_from_user_eabi_flock64(struct flock64 *fl,
> +                                                   abi_ulong target_flock_addr)
> +{
> +    struct target_eabi_flock64 *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    __get_user(l_type, &target_fl->l_type);
> +    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> +    __get_user(fl->l_whence, &target_fl->l_whence);
> +    __get_user(fl->l_start, &target_fl->l_start);
> +    __get_user(fl->l_len, &target_fl->l_len);
> +    __get_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 0);
> +    return 0;
> +}
> +
> +static inline abi_long copy_to_user_eabi_flock64(abi_ulong target_flock_addr,
> +                                                 const struct flock64 *fl)
> +{
> +    struct target_eabi_flock64 *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> +    __put_user(l_type, &target_fl->l_type);
> +    __put_user(fl->l_whence, &target_fl->l_whence);
> +    __put_user(fl->l_start, &target_fl->l_start);
> +    __put_user(fl->l_len, &target_fl->l_len);
> +    __put_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 1);
> +    return 0;
> +}
> +#endif
> +
> +static inline abi_long copy_from_user_flock64(struct flock64 *fl,
> +                                              abi_ulong target_flock_addr)
> +{
> +    struct target_flock64 *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_READ, target_fl, target_flock_addr, 1)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    __get_user(l_type, &target_fl->l_type);
> +    fl->l_type = target_to_host_bitmask(l_type, flock_tbl);
> +    __get_user(fl->l_whence, &target_fl->l_whence);
> +    __get_user(fl->l_start, &target_fl->l_start);
> +    __get_user(fl->l_len, &target_fl->l_len);
> +    __get_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 0);
> +    return 0;
> +}
> +
> +static inline abi_long copy_to_user_flock64(abi_ulong target_flock_addr,
> +                                            const struct flock64 *fl)
> +{
> +    struct target_flock64 *target_fl;
> +    short l_type;
> +
> +    if (!lock_user_struct(VERIFY_WRITE, target_fl, target_flock_addr, 0)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    l_type = host_to_target_bitmask(fl->l_type, flock_tbl);
> +    __put_user(l_type, &target_fl->l_type);
> +    __put_user(fl->l_whence, &target_fl->l_whence);
> +    __put_user(fl->l_start, &target_fl->l_start);
> +    __put_user(fl->l_len, &target_fl->l_len);
> +    __put_user(fl->l_pid, &target_fl->l_pid);
> +    unlock_user_struct(target_fl, target_flock_addr, 1);
> +    return 0;
> +}
> +
> +static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
> +{
>      struct flock64 fl64;
> -    struct target_flock64 *target_fl64;
>  #ifdef F_GETOWN_EX
>      struct f_owner_ex fox;
>      struct target_f_owner_ex *target_fox;
> @@ -4967,77 +5089,41 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>  
>      switch(cmd) {
>      case TARGET_F_GETLK:
> -        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
> +        if (copy_from_user_flock(&fl64, arg)) {
>              return -TARGET_EFAULT;

why do you ignore the exact value returned by copy_from_user_flock()?
You should return this value instead of guessing it.

> -        fl.l_type =
> -                  target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
> -        fl.l_whence = tswap16(target_fl->l_whence);
> -        fl.l_start = tswapal(target_fl->l_start);
> -        fl.l_len = tswapal(target_fl->l_len);
> -        fl.l_pid = tswap32(target_fl->l_pid);
> -        unlock_user_struct(target_fl, arg, 0);
> -        ret = get_errno(fcntl(fd, host_cmd, &fl));
> +        }
> +        ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
> -            if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
> +            if (copy_to_user_flock(arg, &fl64)) {
>                  return -TARGET_EFAULT;
> -            target_fl->l_type =
> -                          host_to_target_bitmask(tswap16(fl.l_type), flock_tbl);
> -            target_fl->l_whence = tswap16(fl.l_whence);
> -            target_fl->l_start = tswapal(fl.l_start);
> -            target_fl->l_len = tswapal(fl.l_len);
> -            target_fl->l_pid = tswap32(fl.l_pid);
> -            unlock_user_struct(target_fl, arg, 1);
> +            }
>          }
>          break;
>  
>      case TARGET_F_SETLK:
>      case TARGET_F_SETLKW:
> -        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
> +        if (copy_from_user_flock(&fl64, arg)) {
>              return -TARGET_EFAULT;

same as above

> -        fl.l_type =
> -                  target_to_host_bitmask(tswap16(target_fl->l_type), flock_tbl);
> -        fl.l_whence = tswap16(target_fl->l_whence);
> -        fl.l_start = tswapal(target_fl->l_start);
> -        fl.l_len = tswapal(target_fl->l_len);
> -        fl.l_pid = tswap32(target_fl->l_pid);
> -        unlock_user_struct(target_fl, arg, 0);
> -        ret = get_errno(fcntl(fd, host_cmd, &fl));
> +        }
> +        ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          break;
>  
>      case TARGET_F_GETLK64:
> -        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> +        if (copy_from_user_flock64(&fl64, arg)) {
>              return -TARGET_EFAULT;

same as above

> -        fl64.l_type =
> -           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;

The ">> 1" disappears...

> -        fl64.l_whence = tswap16(target_fl64->l_whence);
> -        fl64.l_start = tswap64(target_fl64->l_start);
> -        fl64.l_len = tswap64(target_fl64->l_len);
> -        fl64.l_pid = tswap32(target_fl64->l_pid);
> -        unlock_user_struct(target_fl64, arg, 0);
> +        }
>          ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
> -            if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
> +            if (copy_to_user_flock64(arg, &fl64)) {
>                  return -TARGET_EFAULT;

same as above

> -            target_fl64->l_type =
> -                   host_to_target_bitmask(tswap16(fl64.l_type), flock_tbl) >> 1;
> -            target_fl64->l_whence = tswap16(fl64.l_whence);
> -            target_fl64->l_start = tswap64(fl64.l_start);
> -            target_fl64->l_len = tswap64(fl64.l_len);
> -            target_fl64->l_pid = tswap32(fl64.l_pid);
> -            unlock_user_struct(target_fl64, arg, 1);
> +            }
>          }
>          break;
>      case TARGET_F_SETLK64:
>      case TARGET_F_SETLKW64:
> -        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> +        if (copy_from_user_flock64(&fl64, arg)) {
>              return -TARGET_EFAULT;

same as above

> -        fl64.l_type =
> -           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
> -        fl64.l_whence = tswap16(target_fl64->l_whence);
> -        fl64.l_start = tswap64(target_fl64->l_start);
> -        fl64.l_len = tswap64(target_fl64->l_len);
> -        fl64.l_pid = tswap32(target_fl64->l_pid);
> -        unlock_user_struct(target_fl64, arg, 0);
> +        }
>          ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          break;
>  
> @@ -9485,9 +9571,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>      {
>  	int cmd;
>  	struct flock64 fl;
> -	struct target_flock64 *target_fl;
> +        from_flock64_fn *copyfrom = copy_from_user_flock64;
> +        to_flock64_fn *copyto = copy_to_user_flock64;
> +
>  #ifdef TARGET_ARM
> -	struct target_eabi_flock64 *target_efl;
> +        if (((CPUARMState *)cpu_env)->eabi) {
> +            copyfrom = copy_from_user_eabi_flock64;
> +            copyto = copy_to_user_eabi_flock64;
> +        }
>  #endif
>  
>  	cmd = target_to_host_fcntl_cmd(arg2);
> @@ -9498,78 +9589,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  
>          switch(arg2) {
>          case TARGET_F_GETLK64:
> -#ifdef TARGET_ARM
> -            if (((CPUARMState *)cpu_env)->eabi) {
> -                if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
> -                    goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> -                fl.l_whence = tswap16(target_efl->l_whence);
> -                fl.l_start = tswap64(target_efl->l_start);
> -                fl.l_len = tswap64(target_efl->l_len);
> -                fl.l_pid = tswap32(target_efl->l_pid);
> -                unlock_user_struct(target_efl, arg3, 0);
> -            } else
> -#endif
> -            {
> -                if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
> -                    goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> -                fl.l_whence = tswap16(target_fl->l_whence);
> -                fl.l_start = tswap64(target_fl->l_start);
> -                fl.l_len = tswap64(target_fl->l_len);
> -                fl.l_pid = tswap32(target_fl->l_pid);
> -                unlock_user_struct(target_fl, arg3, 0);
> +            if (copyfrom(&fl, arg3)) {
> +                goto efault;
>              }
>              ret = get_errno(fcntl(arg1, cmd, &fl));
>  	    if (ret == 0) {
> -#ifdef TARGET_ARM
> -                if (((CPUARMState *)cpu_env)->eabi) {
> -                    if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 0)) 
> -                        goto efault;
> -                    target_efl->l_type = tswap16(fl.l_type);
> -                    target_efl->l_whence = tswap16(fl.l_whence);
> -                    target_efl->l_start = tswap64(fl.l_start);
> -                    target_efl->l_len = tswap64(fl.l_len);
> -                    target_efl->l_pid = tswap32(fl.l_pid);
> -                    unlock_user_struct(target_efl, arg3, 1);
> -                } else
> -#endif
> -                {
> -                    if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
> -                        goto efault;
> -                    target_fl->l_type = tswap16(fl.l_type);
> -                    target_fl->l_whence = tswap16(fl.l_whence);
> -                    target_fl->l_start = tswap64(fl.l_start);
> -                    target_fl->l_len = tswap64(fl.l_len);
> -                    target_fl->l_pid = tswap32(fl.l_pid);
> -                    unlock_user_struct(target_fl, arg3, 1);
> +                if (copyto(arg3, &fl)) {
> +                    goto efault;
>                  }
>  	    }
>  	    break;
>  
>          case TARGET_F_SETLK64:
>          case TARGET_F_SETLKW64:
> -#ifdef TARGET_ARM
> -            if (((CPUARMState *)cpu_env)->eabi) {
> -                if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
> -                    goto efault;
> -                fl.l_type = tswap16(target_efl->l_type);
> -                fl.l_whence = tswap16(target_efl->l_whence);
> -                fl.l_start = tswap64(target_efl->l_start);
> -                fl.l_len = tswap64(target_efl->l_len);
> -                fl.l_pid = tswap32(target_efl->l_pid);
> -                unlock_user_struct(target_efl, arg3, 0);
> -            } else
> -#endif
> -            {
> -                if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
> -                    goto efault;
> -                fl.l_type = tswap16(target_fl->l_type);
> -                fl.l_whence = tswap16(target_fl->l_whence);
> -                fl.l_start = tswap64(target_fl->l_start);
> -                fl.l_len = tswap64(target_fl->l_len);
> -                fl.l_pid = tswap32(target_fl->l_pid);
> -                unlock_user_struct(target_fl, arg3, 0);
> +            if (copyfrom(&fl, arg3)) {
> +                goto efault;
>              }
>              ret = get_errno(fcntl(arg1, cmd, &fl));
>  	    break;
> 

Re: [Qemu-devel] [PATCH 14/18] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()

[Peter Maydell]

On 7 June 2016 at 21:41, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 06/06/2016 à 20:58, Peter Maydell a écrit :
>> Use the __get_user() and __put_user() to handle reading and writing the
>> guest structures in do_ioctl(). This has two benefits:
>>  * avoids possible errors due to misaligned guest pointers
>>  * correctly sign extends signed fields (like l_start in struct flock)
>>    which might be different sizes between guest and host
>>
>> To do this we abstract out into copy_from/to_user functions. We
>> also standardize on always using host flock64 and the F_GETLK64
>> etc flock commands, as this means we always have 64 bit offsets
>> whether the host is 64-bit or 32-bit and we don't need to support
>> conversion to both host struct flock and struct flock64.
>>
>> In passing we fix errors in converting l_type from the host to
>> the target (where we were doing a byteswap of the host value
>> before trying to do the convert-bitmasks operation rather than
>> otherwise, and inexplicably shifting left by 1).
>
> I  think the ">> 1" is coming from:
>
> 43f238d Support fcntl F_GETLK64, F_SETLK64, F_SETLKW64
>
> to convert arm to x86, and should have been removed then in:
>
> 2ba7f73 alpha-linux-user: Translate fcntl l_type
>
> So yes, the ">> 1" is wrong. I don't understand how it can work.

Thanks for tracking down where it came from. I suspect it
just didn't work and nobody noticed, because:
 * there's not much use of big-on-little-endian
 * a lot of the time the bug is just going to downgrade an
   exclusive lock to a shared lock, and you won't notice if
   there isn't actually any contention on the lock...

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  linux-user/syscall.c | 280 +++++++++++++++++++++++++++++----------------------
>>  1 file changed, 157 insertions(+), 123 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 4cf67c8..f3a487e 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4894,11 +4894,11 @@ static int target_to_host_fcntl_cmd(int cmd)
>>       case TARGET_F_SETFL:
>>              return cmd;
>>          case TARGET_F_GETLK:
>> -         return F_GETLK;
>> -     case TARGET_F_SETLK:
>> -         return F_SETLK;
>> -     case TARGET_F_SETLKW:
>> -         return F_SETLKW;
>> +            return F_GETLK64;
>> +        case TARGET_F_SETLK:
>> +            return F_SETLK64;
>> +        case TARGET_F_SETLKW:
>> +            return F_SETLKW64;
>>       case TARGET_F_GETOWN:
>>           return F_GETOWN;
>>       case TARGET_F_SETOWN:
>
> I see no reason to have this in this patch.

The idea is that we want to use only one host flock struct,
which means it must be the one which supports 64-bit offsets.
On a 32-bit host, that's the flock64 struct, which must be
used with the F_GETLK64 fcntl, not F_GETLK.
On a 64-bit host, the system headers define that F_GETLK64
and F_GETLK are identical (and that the flock64 struct is flock),
so instead of having to specialcase 64-bit hosts, we can just
say "use the F_*64 constants and struct flock64 everywhere".

If we didn't have this hunk of the patch then on a 32-bit
host the code below would go wrong, because when we did
a guest F_GETLK we'd end up passing a (host) struct flock64
to the 32-bit F_GETLK.

>>      case TARGET_F_GETLK:
>> -        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
>> +        if (copy_from_user_flock(&fl64, arg)) {
>>              return -TARGET_EFAULT;
>
> why do you ignore the exact value returned by copy_from_user_flock()?
> You should return this value instead of guessing it.

Yeah, I was being lazy and not wanting to have an extra 'ret'
variable floating around. I'll fix this.

>> -        fl64.l_type =
>> -           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
>
> The ">> 1" disappears...

...and it's correct that it disappears, right?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 18/18] linux-user: Special-case ERESTARTSYS in target_strerror()

[Peter Maydell]

On 7 June 2016 at 20:53, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 06/06/2016 à 20:58, Peter Maydell a écrit :
>> Since TARGET_ERESTARTSYS and TARGET_ESIGRETURN are internal-to-QEMU
>> error numbers, handle them specially in target_strerror(), to avoid
>> confusing strace output like:
>>
>> 9521 rt_sigreturn(14,8,274886297808,8,0,268435456) = -1 errno=513 (Unknown error 513)
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  linux-user/syscall.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index bcee02d..782d475 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -619,6 +619,13 @@ static inline int is_error(abi_long ret)
>>
>>  const char *target_strerror(int err)
>>  {
>> +    if (err == TARGET_ERESTARTSYS) {
>> +        return "To be restarted";
>> +    }
>> +    if (err == TARGET_QEMU_ESIGRETURN) {
>> +        return "Successful exit from sigreturn";
>> +    }
>> +
>>      if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
>>          return NULL;
>>      }
>
> This is not the aim of this patch, but target_to_host_errno() has now
> these checks, perhaps we can remove this while we are here...

I think that would break the callers, which assume they can
pass in any number as a potential errno, and get
back NULL if it wasn't actually an errno. If we passed
them through to target_to_host_errno() it would pass
them on unchanged and the host strerror() would generate
a string "Unknown errno 134134234" or whatever.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/18] linux-user: Extend safe_syscall wrapper use

[Riku Voipio]

On Mon, Jun 06, 2016 at 07:58:01PM +0100, Peter Maydell wrote:
> This set of pretty dull patches extends the use of the safe_syscall
> wrapper to every syscall listed in the signal(7) manpage as being
> interruptible.
>
> Most of the patches are just straightforward "use the wrapper" changes.
> For a few things get a little more complicated because we need to use
> the direct syscall rather than the helpful libc wrapper:
>  * for the IPC syscalls we need to handle the host kernel maybe
>    doing these via individual syscalls and maybe via the single
>    'ipc' syscall
>  * poll has to be implemented via the ppoll syscall now, which means
>    converting the timeout argument
>  * in order to have all the fcntl-related syscalls go via fcntl64
>    (rather than using a wrapper for both fcntl and fcntl64 on 32-bit
>    hosts), there are patches which clean up the conversion of the
>    target_flock data structures. These in passing fix buggy conversion
>    code which was making us fail some LTP tests
> 
> The last two patches are trivial ones which neaten up the QEMU
> strace output for the cases where the returned errnos printed by
> the strace layer are QEMU-internal ones rather than real guest errnos.
> 
> This patchset sits on top of:
>  * the 'fix various signal race conditions' patchset currently on list:
>    https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg05057.html
>  * the fadvise patches (on list, reviewed)
>  * the 'provide frame information in x86-64 safe_syscall' patch v2
>    (on list, reviewed)
> 
> https://git.linaro.org/people/peter.maydell/qemu-arm.git sigrace-fixes-3
> is a git branch with those prequisites plus this patchset.

I've merged most of this now to to:

https://git.linaro.org/people/riku.voipio/qemu.git/shortlog/refs/heads/linux-user-for-upstream

With the changes of Adding Laurent's reviewed by's where given and
And adjusted changed to "abi_llong" in "Correct signedness of target_flock l_start and l_len fields" as suggested. Dropped:

- linux-user: Use __get_user() and __put_user() to handle...
    Due to comments
  linux-user: Use safe_syscall wrapper for fcntl
    Because it needs the above one
  linux-user: Avoid possible misalignment in host_to_target_siginfo()
    Since it break's compile

I think I'd prefer to send this set as-is (after some more testing) and
get fcntl and siginfo fixed for next batch. This set is already quite
big and I guess people are eager to test this fixes too.

Riku

> thanks
> -- PMM
> 
> Peter Maydell (18):
>   linux-user: Use safe_syscall wrapper for readv and writev syscalls
>   linux-user: Use safe_syscall wrapper for connect syscall
>   linux-user: Use safe_syscall wrapper for send* and recv* syscalls
>   linux-user: Use safe_syscall wrapper for msgsnd and msgrcv
>   linux-user: Use safe_syscall wrapper for mq_timedsend and
>     mq_timedreceive
>   linux-user: Use safe_syscall wrapper for flock
>   linux-user: Use safe_syscall wrapper for rt_sigtimedwait syscall
>   linux-user: Use safe_syscall wrapper for sleep syscalls
>   linux-user: Use safe_syscall wrapper for poll and ppoll syscalls
>   linux-user: Use safe_syscall wrapper for epoll_wait syscalls
>   linux-user: Use safe_syscall wrapper for semop
>   linux-user: Use safe_syscall wrapper for accept and accept4 syscalls
>   linux-user: Use safe_syscall wrapper for ioctl
>   linux-user: Use __get_user() and __put_user() to handle structs in
>     do_fcntl()
>   linux-user: Correct signedness of target_flock l_start and l_len
>     fields
>   linux-user: Use safe_syscall wrapper for fcntl
>   linux-user: Make target_strerror() return 'const char *'
>   linux-user: Special-case ERESTARTSYS in target_strerror()
> 
>  configure                 |  21 +-
>  linux-user/qemu.h         |   2 +-
>  linux-user/strace.c       |   4 +-
>  linux-user/syscall.c      | 547 ++++++++++++++++++++++++++++------------------
>  linux-user/syscall_defs.h |  34 +--
>  5 files changed, 359 insertions(+), 249 deletions(-)
> 
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 14/18] linux-user: Use __get_user() and __put_user() to handle structs in do_fcntl()

[Laurent Vivier]

Le 07/06/2016 à 23:20, Peter Maydell a écrit :
> On 7 June 2016 at 21:41, Laurent Vivier <laurent@vivier.eu> wrote:
>>> -        fl64.l_type =
>>> -           target_to_host_bitmask(tswap16(target_fl64->l_type), flock_tbl) >> 1;
>>
>> The ">> 1" disappears...
> 
> ...and it's correct that it disappears, right?

Yes. I forgot to remove this when I have understood what was the reason
of the shift.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 00/18] linux-user: Extend safe_syscall wrapper use

[Peter Maydell]

On 8 June 2016 at 10:20, Riku Voipio <riku.voipio@iki.fi> wrote:
> I've merged most of this now to to:
>
> https://git.linaro.org/people/riku.voipio/qemu.git/shortlog/refs/heads/linux-user-for-upstream
>
> With the changes of Adding Laurent's reviewed by's where given and
> And adjusted changed to "abi_llong" in "Correct signedness of target_flock l_start and l_len fields" as suggested. Dropped:
>
> - linux-user: Use __get_user() and __put_user() to handle...
>     Due to comments
>   linux-user: Use safe_syscall wrapper for fcntl
>     Because it needs the above one
>   linux-user: Avoid possible misalignment in host_to_target_siginfo()
>     Since it break's compile
>
> I think I'd prefer to send this set as-is (after some more testing) and
> get fcntl and siginfo fixed for next batch. This set is already quite
> big and I guess people are eager to test this fixes too.

That's great, thanks. I'll sort out those dropped patches
and resend fixed versions.

-- PMM