Re: [Qemu-devel] [PATCH] linux-user: Emulate SOCK_CLOEXEC/NONBLOCK if unavailable

[Riku Voipio <riku.voipio@iki.fi>]

Hi,

On Mon, Sep 16, 2013 at 03:08:06PM +0200, edgar.iglesias@gmail.com wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> 
> If the host lacks support for SOCK_CLOEXEC or SOCK_NONBLOCK,
> try to emulate them with fcntl() FD_CLOEXEC and O_NONBLOCK.

Last time emulating CLOEXEC with fcntl was discussed, the idea
was rejected[1]. The whole point of CLOEXEC flag is guarantee
race free open, which when implemented this way it is no longer.

It is better to tell the userspace that atomic operation is not
available and let the userspace app/library to cope with that,
than give the application false sense of safety.

Also, this was 4 years ago, on which platforms is this still
important?

Riku

[1] http://lists.nongnu.org/archive/html/qemu-devel/2009-05/msg00263.html

> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  linux-user/syscall.c |   48 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index c62d875..6aa8cd7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1701,7 +1701,7 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
>      free(vec);
>  }
>  
> -static inline void target_to_host_sock_type(int *type)
> +static inline int target_to_host_sock_type(int *type)
>  {
>      int host_type = 0;
>      int target_type = *type;
> @@ -1718,22 +1718,64 @@ static inline void target_to_host_sock_type(int *type)
>          break;
>      }
>      if (target_type & TARGET_SOCK_CLOEXEC) {
> +#if defined(SOCK_CLOEXEC)
>          host_type |= SOCK_CLOEXEC;
> +#elif !defined(FD_CLOEXEC)
> +        return -TARGET_EINVAL;
> +#endif
>      }
>      if (target_type & TARGET_SOCK_NONBLOCK) {
> +#if defined(SOCK_NONBLOCK)
>          host_type |= SOCK_NONBLOCK;
> +#elif !defined(O_NONBLOCK)
> +        return -TARGET_EINVAL;
> +#endif
>      }
>      *type = host_type;
> +    return 0;
> +}
> +
> +/* Try to emulate socket type flags after socket creation.  */
> +static int sock_flags_fixup(int fd, int target_type)
> +{
> +#if !defined(SOCK_CLOEXEC) && defined(FD_CLOEXEC)
> +    if (target_type & TARGET_SOCK_CLOEXEC) {
> +        if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
> +            close(fd);
> +            return -TARGET_EINVAL;
> +        }
> +    }
> +#endif
> +#if !defined(SOCK_NONBLOCK) && defined(O_NONBLOCK)
> +    if (target_type & TARGET_SOCK_NONBLOCK) {
> +        int flags = fcntl(fd, F_GETFL);
> +        if (fcntl(fd, F_SETFL, O_NONBLOCK | flags) == -1) {
> +            close(fd);
> +            return -TARGET_EINVAL;
> +        }
> +    }
> +#endif
> +    return fd;
>  }
>  
>  /* do_socket() Must return target values and target errnos. */
>  static abi_long do_socket(int domain, int type, int protocol)
>  {
> -    target_to_host_sock_type(&type);
> +    int target_type = type;
> +    int ret;
> +
> +    ret = target_to_host_sock_type(&type);
> +    if (ret) {
> +        return ret;
> +    }
>  
>      if (domain == PF_NETLINK)
>          return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> -    return get_errno(socket(domain, type, protocol));
> +    ret = get_errno(socket(domain, type, protocol));
> +    if (ret >= 0) {
> +        ret = sock_flags_fixup(ret, target_type);
> +    }
> +    return ret;
>  }
>  
>  /* do_bind() Must return target values and target errnos. */
> -- 
> 1.7.10.4