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