[Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl()

[Qemu-devel] [PATCH v2 0/3] Use safe_syscall wrapper for fcntl()

[Peter Maydell]

This is a fixed respin of the three patches which didn't make
it through review from my last set of linux-user patches.

Changes v1->v2:
 * add a memset() to host_to_target_siginfo_noswap() to suppress
   bogus gcc warnings about struct fields being maybe used while
   uninitialized
 * pass the error returned from copy_to/from_user_flock() etc out to
   the guest rather than assuming it's always -TARGET_EFAULT
 * added a note to the commit message about where the bogus shift
   operations in conversion of l_type were from

thanks
-- PMM


Peter Maydell (3):
  linux-user: Avoid possible misalignment in host_to_target_siginfo()
  linux-user: Use __get_user() and __put_user() to handle structs in
    do_fcntl()
  linux-user: Use safe_syscall wrapper for fcntl

 linux-user/signal.c  |  13 +-
 linux-user/syscall.c | 328 +++++++++++++++++++++++++++++----------------------
 2 files changed, 198 insertions(+), 143 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo()

[Peter Maydell]

host_to_target_siginfo() is implemented by a combination of
host_to_target_siginfo_noswap() followed by tswap_siginfo().
The first of these two functions assumes that the target_siginfo_t
it is writing to is correctly aligned, but the pointer passed
into host_to_target_siginfo() is directly from the guest and
might be misaligned. Use a local variable to avoid this problem.
(tswap_siginfo() does now correctly handle a misaligned destination.)

We have to add a memset() to host_to_target_siginfo_noswap()
to avoid some false positive "may be used uninitialized" warnings
from gcc about subfields of the _sifields union if it chooses to
inline both tswap_siginfo() and host_to_target_siginfo_noswap()
into host_to_target_siginfo().

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

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 61c1145..37fb60f 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -281,6 +281,14 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
     tinfo->si_errno = 0;
     tinfo->si_code = info->si_code;
 
+    /* This memset serves two purposes:
+     * (1) ensure we don't leak random junk to the guest later
+     * (2) placate false positives from gcc about fields
+     *     being used uninitialized if it chooses to inline both this
+     *     function and tswap_siginfo() into host_to_target_siginfo().
+     */
+    memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));
+
     /* This is awkward, because we have to use a combination of
      * the si_code and si_signo to figure out which of the union's
      * members are valid. (Within the host kernel it is always possible
@@ -400,8 +408,9 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
 
 void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info)
 {
-    host_to_target_siginfo_noswap(tinfo, info);
-    tswap_siginfo(tinfo, tinfo);
+    target_siginfo_t tgt_tmp;
+    host_to_target_siginfo_noswap(&tgt_tmp, info);
+    tswap_siginfo(tinfo, &tgt_tmp);
 }
 
 /* XXX: we support only POSIX RT signals are used. */
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Avoid possible misalignment in host_to_target_siginfo()

[Laurent Vivier]

Le 09/06/2016 à 16:09, Peter Maydell a écrit :
> host_to_target_siginfo() is implemented by a combination of
> host_to_target_siginfo_noswap() followed by tswap_siginfo().
> The first of these two functions assumes that the target_siginfo_t
> it is writing to is correctly aligned, but the pointer passed
> into host_to_target_siginfo() is directly from the guest and
> might be misaligned. Use a local variable to avoid this problem.
> (tswap_siginfo() does now correctly handle a misaligned destination.)
> 
> We have to add a memset() to host_to_target_siginfo_noswap()
> to avoid some false positive "may be used uninitialized" warnings
> from gcc about subfields of the _sifields union if it chooses to
> inline both tswap_siginfo() and host_to_target_siginfo_noswap()
> into host_to_target_siginfo().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  linux-user/signal.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 61c1145..37fb60f 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -281,6 +281,14 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>      tinfo->si_errno = 0;
>      tinfo->si_code = info->si_code;
>  
> +    /* This memset serves two purposes:
> +     * (1) ensure we don't leak random junk to the guest later
> +     * (2) placate false positives from gcc about fields
> +     *     being used uninitialized if it chooses to inline both this
> +     *     function and tswap_siginfo() into host_to_target_siginfo().
> +     */
> +    memset(tinfo->_sifields._pad, 0, sizeof(tinfo->_sifields._pad));
> +
>      /* This is awkward, because we have to use a combination of
>       * the si_code and si_signo to figure out which of the union's
>       * members are valid. (Within the host kernel it is always possible
> @@ -400,8 +408,9 @@ static void tswap_siginfo(target_siginfo_t *tinfo,
>  
>  void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info)
>  {
> -    host_to_target_siginfo_noswap(tinfo, info);
> -    tswap_siginfo(tinfo, tinfo);
> +    target_siginfo_t tgt_tmp;
> +    host_to_target_siginfo_noswap(&tgt_tmp, info);
> +    tswap_siginfo(tinfo, &tgt_tmp);
>  }
>  
>  /* XXX: we support only POSIX RT signals are used. */
> 

[Qemu-devel] [PATCH v2 2/3] 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); these were
accidentally left over when the original simple "just shift by 1"
arm<->x86 conversion of commit 43f238d was changed to the more
general scheme of using target_to_host_bitmask() functions in 2ba7f73.

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

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 71ccbd9..78c36dd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5542,11 +5542,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:
@@ -5597,12 +5597,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;
@@ -5615,77 +5737,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))
-            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 = copy_from_user_flock(&fl64, arg);
+        if (ret) {
+            return ret;
+        }
+        ret = get_errno(fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
-            if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
-                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);
+            ret = copy_to_user_flock(arg, &fl64);
         }
         break;
 
     case TARGET_F_SETLK:
     case TARGET_F_SETLKW:
-        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
-            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 = copy_from_user_flock(&fl64, arg);
+        if (ret) {
+            return ret;
+        }
+        ret = get_errno(fcntl(fd, host_cmd, &fl64));
         break;
 
     case TARGET_F_GETLK64:
-        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
-            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 = copy_from_user_flock64(&fl64, arg);
+        if (ret) {
+            return ret;
+        }
         ret = get_errno(fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
-            if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
-                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);
+            ret = copy_to_user_flock64(arg, &fl64);
         }
         break;
     case TARGET_F_SETLK64:
     case TARGET_F_SETLKW64:
-        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
-            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 = copy_from_user_flock64(&fl64, arg);
+        if (ret) {
+            return ret;
+        }
         ret = get_errno(fcntl(fd, host_cmd, &fl64));
         break;
 
@@ -10133,9 +10219,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);
@@ -10146,78 +10237,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);
+            ret = copyfrom(&fl, arg3);
+            if (ret) {
+                break;
             }
             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 (ret == 0) {
+                ret = copyto(arg3, &fl);
+            }
 	    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);
+            ret = copyfrom(&fl, arg3);
+            if (ret) {
+                break;
             }
             ret = get_errno(fcntl(arg1, cmd, &fl));
 	    break;
-- 
1.9.1

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

[Laurent Vivier]

Le 09/06/2016 à 16:09, 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); these were
> accidentally left over when the original simple "just shift by 1"
> arm<->x86 conversion of commit 43f238d was changed to the more
> general scheme of using target_to_host_bitmask() functions in 2ba7f73.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

> ---
>  linux-user/syscall.c | 298 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 166 insertions(+), 132 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 71ccbd9..78c36dd 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5542,11 +5542,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:
> @@ -5597,12 +5597,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;
> @@ -5615,77 +5737,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))
> -            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 = copy_from_user_flock(&fl64, arg);
> +        if (ret) {
> +            return ret;
> +        }
> +        ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
> -            if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
> -                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);
> +            ret = copy_to_user_flock(arg, &fl64);
>          }
>          break;
>  
>      case TARGET_F_SETLK:
>      case TARGET_F_SETLKW:
> -        if (!lock_user_struct(VERIFY_READ, target_fl, arg, 1))
> -            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 = copy_from_user_flock(&fl64, arg);
> +        if (ret) {
> +            return ret;
> +        }
> +        ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          break;
>  
>      case TARGET_F_GETLK64:
> -        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> -            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 = copy_from_user_flock64(&fl64, arg);
> +        if (ret) {
> +            return ret;
> +        }
>          ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
> -            if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
> -                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);
> +            ret = copy_to_user_flock64(arg, &fl64);
>          }
>          break;
>      case TARGET_F_SETLK64:
>      case TARGET_F_SETLKW64:
> -        if (!lock_user_struct(VERIFY_READ, target_fl64, arg, 1))
> -            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 = copy_from_user_flock64(&fl64, arg);
> +        if (ret) {
> +            return ret;
> +        }
>          ret = get_errno(fcntl(fd, host_cmd, &fl64));
>          break;
>  
> @@ -10133,9 +10219,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);
> @@ -10146,78 +10237,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);
> +            ret = copyfrom(&fl, arg3);
> +            if (ret) {
> +                break;
>              }
>              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 (ret == 0) {
> +                ret = copyto(arg3, &fl);
> +            }
>  	    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);
> +            ret = copyfrom(&fl, arg3);
> +            if (ret) {
> +                break;
>              }
>              ret = get_errno(fcntl(arg1, cmd, &fl));
>  	    break;
> 

[Qemu-devel] [PATCH v2 3/3] 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 78c36dd..121b89f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -784,6 +784,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)
 {
@@ -5741,7 +5751,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (ret) {
             return ret;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
             ret = copy_to_user_flock(arg, &fl64);
         }
@@ -5753,7 +5763,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (ret) {
             return ret;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         break;
 
     case TARGET_F_GETLK64:
@@ -5761,7 +5771,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (ret) {
             return ret;
         }
-        ret = get_errno(fcntl(fd, host_cmd, &fl64));
+        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
         if (ret == 0) {
             ret = copy_to_user_flock64(arg, &fl64);
         }
@@ -5772,23 +5782,25 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
         if (ret) {
             return ret;
         }
-        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;
@@ -5806,7 +5818,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
 
@@ -5816,11 +5828,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;
@@ -10253,7 +10265,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (ret) {
                 break;
             }
-            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

Re: [Qemu-devel] [PATCH v2 3/3] linux-user: Use safe_syscall wrapper for fcntl

[Laurent Vivier]

Le 09/06/2016 à 16:09, Peter Maydell a écrit :
> 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>

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

> ---
>  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 78c36dd..121b89f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -784,6 +784,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)
>  {
> @@ -5741,7 +5751,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          if (ret) {
>              return ret;
>          }
> -        ret = get_errno(fcntl(fd, host_cmd, &fl64));
> +        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
>              ret = copy_to_user_flock(arg, &fl64);
>          }
> @@ -5753,7 +5763,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          if (ret) {
>              return ret;
>          }
> -        ret = get_errno(fcntl(fd, host_cmd, &fl64));
> +        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
>          break;
>  
>      case TARGET_F_GETLK64:
> @@ -5761,7 +5771,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          if (ret) {
>              return ret;
>          }
> -        ret = get_errno(fcntl(fd, host_cmd, &fl64));
> +        ret = get_errno(safe_fcntl(fd, host_cmd, &fl64));
>          if (ret == 0) {
>              ret = copy_to_user_flock64(arg, &fl64);
>          }
> @@ -5772,23 +5782,25 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          if (ret) {
>              return ret;
>          }
> -        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;
> @@ -5806,7 +5818,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
>  
> @@ -5816,11 +5828,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;
> @@ -10253,7 +10265,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (ret) {
>                  break;
>              }
> -            ret = get_errno(fcntl(arg1, cmd, &fl));
> +            ret = get_errno(safe_fcntl(arg1, cmd, &fl));
>  	    break;
>          default:
>              ret = do_fcntl(arg1, arg2, arg3);
>