diff mbox

linux-user: Emulate SOCK_CLOEXEC/NONBLOCK if unavailable

Message ID 1379336886-14715-1-git-send-email-edgar.iglesias@gmail.com
State New
Headers show

Commit Message

Edgar E. Iglesias Sept. 16, 2013, 1:08 p.m. UTC
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.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
---
 linux-user/syscall.c |   48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

Comments

Riku Voipio Sept. 23, 2013, 9:37 a.m. UTC | #1
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
Edgar E. Iglesias Sept. 23, 2013, 11:28 a.m. UTC | #2
On Mon, Sep 23, 2013 at 12:37:10PM +0300, Riku Voipio wrote:
> Hi,

Hi Riku,

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

Agreed, I'll send a v2 that EINVALs on lack of SOCK_CLOEXEC.
My primary concern is to avoid the build error of qemu.

Thanks,
Edgar
diff mbox

Patch

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. */