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

[Laurent Vivier <laurent@vivier.eu>]

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