[Qemu-devel] [PATCH v1 20/21] osdep: remove direct use of qemu_socket & qemu_accept

["Daniel P. Berrange" <berrange@redhat.com>]

The qemu_socket & qemu_accept wrappers exist to ensure
the O_CLOEXEC flag is always set on new sockets. All
code is supposed to use these methods instead of the
normal POSIX socket/accept methods. This proves rather
error prone though with a number of places forgetting
to call the QEMU specific wrappers.

QEMU is already using a technique to transparently
wrap sockets APIs on Win32, avoiding the need for callers
to remember to use QEMU specific wrappers. This switches
over to that technique for socket/accept on POSIX platforms,
removing the need for callers to use the qemu_socket and
qemu_accept methods directly.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 contrib/ivshmem-server/ivshmem-server.c |  4 ++--
 include/qemu/sockets.h                  |  2 --
 include/sysemu/os-posix.h               | 11 +++++++++
 migration/tcp.c                         |  2 +-
 migration/unix.c                        |  2 +-
 net/socket.c                            | 10 ++++----
 qga/channel-posix.c                     |  4 ++--
 slirp/ip_icmp.c                         |  2 +-
 slirp/misc.c                            |  4 ++--
 slirp/socket.c                          |  2 +-
 slirp/tcp_subr.c                        |  2 +-
 slirp/udp.c                             |  4 ++--
 util/osdep.c                            | 42 ---------------------------------
 util/oslib-posix.c                      | 39 ++++++++++++++++++++++++++++++
 util/oslib-win32.c                      |  4 ++++
 util/qemu-sockets.c                     | 10 ++++----
 16 files changed, 77 insertions(+), 67 deletions(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
index bfd0fad..1b72ca0 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -142,8 +142,8 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
 
     /* accept the incoming connection */
     unaddr_len = sizeof(unaddr);
-    newfd = qemu_accept(server->sock_fd,
-                        (struct sockaddr *)&unaddr, &unaddr_len);
+    newfd = accept(server->sock_fd,
+                   (struct sockaddr *)&unaddr, &unaddr_len);
 
     if (newfd < 0) {
         IVSHMEM_SERVER_DEBUG(server, "cannot accept() %s\n", strerror(errno));
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 1bd9218..331cf5a 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -11,8 +11,6 @@ int inet_aton(const char *cp, struct in_addr *ia);
 #include "qapi-types.h"
 
 /* misc helpers */
-int qemu_socket(int domain, int type, int protocol);
-int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
 void qemu_set_block(int fd);
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 4f529d3..80cc84c 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -56,4 +56,15 @@ int qemu_utimens(const char *path, const qemu_timespec *times);
 
 bool is_daemonized(void);
 
+/* Some sockets wrappers to handle O_CLOEXEC */
+
+#undef socket
+#define socket qemu_socket_wrap
+int qemu_socket_wrap(int domain, int type, int protocol);
+
+#undef accept
+#define accept qemu_accept_wrap
+int qemu_accept_wrap(int sockfd, struct sockaddr *addr,
+                     socklen_t *addrlen);
+
 #endif
diff --git a/migration/tcp.c b/migration/tcp.c
index 886ccfb..ce01e96 100644
--- a/migration/tcp.c
+++ b/migration/tcp.c
@@ -62,7 +62,7 @@ static void tcp_accept_incoming_migration(void *opaque)
     int c;
 
     do {
-        c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
+        c = accept(s, (struct sockaddr *)&addr, &addrlen);
     } while (c < 0 && errno == EINTR);
     qemu_set_fd_handler(s, NULL, NULL, NULL);
     close(s);
diff --git a/migration/unix.c b/migration/unix.c
index d9aac36..b59ac96 100644
--- a/migration/unix.c
+++ b/migration/unix.c
@@ -62,7 +62,7 @@ static void unix_accept_incoming_migration(void *opaque)
     int c, err;
 
     do {
-        c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
+        c = accept(s, (struct sockaddr *)&addr, &addrlen);
         err = errno;
     } while (c < 0 && err == EINTR);
     qemu_set_fd_handler(s, NULL, NULL, NULL);
diff --git a/net/socket.c b/net/socket.c
index b2d7a14..19addba 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -262,7 +262,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr
         return -1;
 
     }
-    fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
+    fd = socket(PF_INET, SOCK_DGRAM, 0);
     if (fd < 0) {
         perror("socket(PF_INET, SOCK_DGRAM)");
         return -1;
@@ -497,7 +497,7 @@ static void net_socket_accept(void *opaque)
 
     for(;;) {
         len = sizeof(saddr);
-        fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
+        fd = accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
         if (fd < 0 && errno != EINTR) {
             return;
         } else if (fd >= 0) {
@@ -527,7 +527,7 @@ static int net_socket_listen_init(NetClientState *peer,
     if (parse_host_port(&saddr, host_str) < 0)
         return -1;
 
-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    fd = socket(PF_INET, SOCK_STREAM, 0);
     if (fd < 0) {
         perror("socket");
         return -1;
@@ -571,7 +571,7 @@ static int net_socket_connect_init(NetClientState *peer,
     if (parse_host_port(&saddr, host_str) < 0)
         return -1;
 
-    fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+    fd = socket(PF_INET, SOCK_STREAM, 0);
     if (fd < 0) {
         perror("socket");
         return -1;
@@ -664,7 +664,7 @@ static int net_socket_udp_init(NetClientState *peer,
         return -1;
     }
 
-    fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
+    fd = socket(PF_INET, SOCK_DGRAM, 0);
     if (fd < 0) {
         perror("socket(PF_INET, SOCK_DGRAM)");
         return -1;
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 7ad3c00..c2d9440 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -31,8 +31,8 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
 
     g_assert(channel != NULL);
 
-    client_fd = qemu_accept(g_io_channel_unix_get_fd(channel),
-                            (struct sockaddr *)&addr, &addrlen);
+    client_fd = accept(g_io_channel_unix_get_fd(channel),
+                       (struct sockaddr *)&addr, &addrlen);
     if (client_fd == -1) {
         g_warning("error converting fd to gsocket: %s", strerror(errno));
         goto out;
diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index 6c7fc63..e465f2e 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -79,7 +79,7 @@ static int icmp_send(struct socket *so, struct mbuf *m, int hlen)
     struct ip *ip = mtod(m, struct ip *);
     struct sockaddr_in addr;
 
-    so->s = qemu_socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP);
+    so->s = socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP);
     if (so->s == -1) {
         return -1;
     }
diff --git a/slirp/misc.c b/slirp/misc.c
index d7e39af..3c62c33 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -135,7 +135,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 		addr.sin_port = 0;
 		addr.sin_addr.s_addr = INADDR_ANY;
 
-		if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
+		if ((s = socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
 		    bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
 		    listen(s, 1) < 0) {
 			error_report("Error: inet socket: %s", strerror(errno));
@@ -162,7 +162,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
                  * Connect to the socket
                  * XXX If any of these fail, we're in trouble!
                  */
-                s = qemu_socket(AF_INET, SOCK_STREAM, 0);
+                s = socket(AF_INET, SOCK_STREAM, 0);
                 addr.sin_addr = loopback_addr;
                 do {
                     ret = connect(s, (struct sockaddr *)&addr, addrlen);
diff --git a/slirp/socket.c b/slirp/socket.c
index f1b9190..e469e69 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -630,7 +630,7 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	addr.sin_addr.s_addr = haddr;
 	addr.sin_port = hport;
 
-	if (((s = qemu_socket(AF_INET,SOCK_STREAM,0)) < 0) ||
+	if (((s = socket(AF_INET, SOCK_STREAM, 0)) < 0) ||
 	    (socket_set_fast_reuse(s) < 0) ||
 	    (bind(s,(struct sockaddr *)&addr, sizeof(addr)) < 0) ||
 	    (listen(s,1) < 0)) {
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 6e07426..dbbe742 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -332,7 +332,7 @@ int tcp_fconnect(struct socket *so, unsigned short af)
   DEBUG_CALL("tcp_fconnect");
   DEBUG_ARG("so = %p", so);
 
-  ret = so->s = qemu_socket(af, SOCK_STREAM, 0);
+  ret = so->s = socket(af, SOCK_STREAM, 0);
   if (ret >= 0) {
     int opt, s=so->s;
     struct sockaddr_storage addr;
diff --git a/slirp/udp.c b/slirp/udp.c
index 6ec884a..f2ca25c 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -280,7 +280,7 @@ int udp_output(struct socket *so, struct mbuf *m,
 int
 udp_attach(struct socket *so, unsigned short af)
 {
-  so->s = qemu_socket(af, SOCK_DGRAM, 0);
+  so->s = socket(af, SOCK_DGRAM, 0);
   if (so->s != -1) {
     so->so_expire = curtime + SO_EXPIRE;
     insque(so, &so->slirp->udb);
@@ -329,7 +329,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
 	if (!so) {
 	    return NULL;
 	}
-	so->s = qemu_socket(AF_INET,SOCK_DGRAM,0);
+	so->s = socket(AF_INET, SOCK_DGRAM, 0);
 	so->so_expire = curtime + SO_EXPIRE;
 	insque(so, &slirp->udb);
 
diff --git a/util/osdep.c b/util/osdep.c
index 8356bdd..27898cf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -267,48 +267,6 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
     return total;
 }
 
-/*
- * Opens a socket with FD_CLOEXEC set
- */
-int qemu_socket(int domain, int type, int protocol)
-{
-    int ret;
-
-#ifdef SOCK_CLOEXEC
-    ret = socket(domain, type | SOCK_CLOEXEC, protocol);
-    if (ret != -1 || errno != EINVAL) {
-        return ret;
-    }
-#endif
-    ret = socket(domain, type, protocol);
-    if (ret >= 0) {
-        qemu_set_cloexec(ret);
-    }
-
-    return ret;
-}
-
-/*
- * Accept a connection and set FD_CLOEXEC
- */
-int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
-{
-    int ret;
-
-#ifdef CONFIG_ACCEPT4
-    ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
-    if (ret != -1 || errno != ENOSYS) {
-        return ret;
-    }
-#endif
-    ret = accept(s, addr, addrlen);
-    if (ret >= 0) {
-        qemu_set_cloexec(ret);
-    }
-
-    return ret;
-}
-
 void qemu_set_hw_version(const char *version)
 {
     hw_version = version;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 7615be4..c6ec8be 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -509,3 +509,42 @@ pid_t qemu_fork(Error **errp)
     }
     return pid;
 }
+
+
+#undef socket
+int qemu_socket_wrap(int domain, int type, int protocol)
+{
+    int ret;
+
+#ifdef SOCK_CLOEXEC
+    ret = socket(domain, type | SOCK_CLOEXEC, protocol);
+    if (ret != -1 || errno != EINVAL) {
+        return ret;
+    }
+#endif
+    ret = socket(domain, type, protocol);
+    if (ret >= 0) {
+        qemu_set_cloexec(ret);
+    }
+
+    return ret;
+}
+
+#undef accept
+int qemu_accept_wrap(int s, struct sockaddr *addr, socklen_t *addrlen)
+{
+    int ret;
+
+#ifdef CONFIG_ACCEPT4
+    ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
+    if (ret != -1 || errno != ENOSYS) {
+        return ret;
+    }
+#endif
+    ret = accept(s, addr, addrlen);
+    if (ret >= 0) {
+        qemu_set_cloexec(ret);
+    }
+
+    return ret;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 77db075..0212360 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -626,6 +626,8 @@ int qemu_socket_wrap(int domain, int type, int protocol)
     ret = socket(domain, type, protocol);
     if (ret < 0) {
         errno = socket_error();
+    } else {
+        qemu_set_cloexec(ret);
     }
     return ret;
 }
@@ -639,6 +641,8 @@ int qemu_accept_wrap(int sockfd, struct sockaddr *addr,
     ret = accept(sockfd, addr, addrlen);
     if (ret < 0) {
         errno = socket_error();
+    } else {
+        qemu_set_cloexec(ret);
     }
     return ret;
 }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index ae3c804..89ef32d 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -180,7 +180,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
         getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
 		        uaddr,INET6_ADDRSTRLEN,uport,32,
 		        NI_NUMERICHOST | NI_NUMERICSERV);
-        slisten = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+        slisten = socket(e->ai_family, e->ai_socktype, e->ai_protocol);
         if (slisten < 0) {
             if (!e->ai_next) {
                 error_setg_errno(errp, errno, "Failed to create socket");
@@ -317,7 +317,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
 
     *in_progress = false;
 
-    sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
+    sock = socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create socket");
         return -1;
@@ -505,7 +505,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
     }
 
     /* create socket */
-    sock = qemu_socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
+    sock = socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create socket");
         goto err;
@@ -694,7 +694,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
     struct sockaddr_un un;
     int sock, fd;
 
-    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create Unix socket");
         return -1;
@@ -767,7 +767,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp,
         return -1;
     }
 
-    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
+    sock = socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create socket");
         return -1;
-- 
2.5.0