Fix a bunch of error handling issues by creating the "TCP socketpair" from the parent process. (the code is currently disabled on Windows: at first I thought a socketpair() could be used, but I realized that slirp calls MSG_OOB on the socket, which isn't implemented on Linux)
Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> --- slirp/misc.c | 133 ++++++++++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 61 deletions(-) diff --git a/slirp/misc.c b/slirp/misc.c index 54edc0b0b9..d9804f2a6d 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -73,42 +73,75 @@ fork_exec(struct socket *so, const char *ex) #else -/* - * XXX This is ugly - * We create and bind a socket, then fork off to another - * process, which connects to this socket, after which we - * exec the wanted program. If something (strange) happens, - * the accept() call could block us forever. - */ +static int +slirp_socketpair_with_oob(int sv[2]) +{ + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_port = 0, + .sin_addr.s_addr = INADDR_ANY, + }; + socklen_t addrlen = sizeof(addr); + int ret, s; + + sv[1] = -1; + s = qemu_socket(AF_INET, SOCK_STREAM, 0); + if (s < 0 || bind(s, (struct sockaddr *)&addr, addrlen) < 0 || + listen(s, 1) < 0 || + getsockname(s, (struct sockaddr *)&addr, &addrlen) < 0) { + goto err; + } + + sv[1] = qemu_socket(AF_INET, SOCK_STREAM, 0); + if (sv[1] < 0) { + goto err; + } + + qemu_set_nonblock(sv[1]); + do { + ret = connect(sv[1], (struct sockaddr *)&addr, addrlen); + } while (ret < 0 && errno == EINTR); + if (ret == -1 && errno != EINPROGRESS) { + goto err; + } + qemu_set_block(sv[1]); + + do { + sv[0] = accept(s, (struct sockaddr *)&addr, &addrlen); + } while (sv[0] < 0 && errno == EINTR); + if (sv[0] < 0) { + goto err; + } + + closesocket(s); + return 0; + +err: + error_report("Error: slirp_socketpair(): %s", strerror(errno)); + if (s >= 0) { + closesocket(s); + } + if (sv[1] >= 0) { + closesocket(sv[1]); + } + return -1; +} + int fork_exec(struct socket *so, const char *ex) { - int s; - struct sockaddr_in addr; - socklen_t addrlen = sizeof(addr); - int opt; const char *argv[256]; /* don't want to clobber the original */ char *bptr; const char *curarg; - int c, i, ret; + int opt, c, i, sp[2]; pid_t pid; DEBUG_CALL("fork_exec"); DEBUG_ARG("so = %p", so); DEBUG_ARG("ex = %p", ex); - addr.sin_family = AF_INET; - addr.sin_port = 0; - addr.sin_addr.s_addr = INADDR_ANY; - - s = qemu_socket(AF_INET, SOCK_STREAM, 0); - if (s < 0 || bind(s, (struct sockaddr *)&addr, addrlen) < 0 || - listen(s, 1) < 0) { - error_report("Error: inet socket: %s", strerror(errno)); - if (s >= 0) { - closesocket(s); - } + if (slirp_socketpair_with_oob(sp) < 0) { return 0; } @@ -116,30 +149,17 @@ fork_exec(struct socket *so, const char *ex) switch(pid) { case -1: error_report("Error: fork failed: %s", strerror(errno)); - close(s); + closesocket(sp[0]); + closesocket(sp[1]); return 0; case 0: - setsid(); - - /* Set the DISPLAY */ - getsockname(s, (struct sockaddr *)&addr, &addrlen); - close(s); - /* - * Connect to the socket - * XXX If any of these fail, we're in trouble! - */ - s = qemu_socket(AF_INET, SOCK_STREAM, 0); - addr.sin_addr = loopback_addr; - do { - ret = connect(s, (struct sockaddr *)&addr, addrlen); - } while (ret < 0 && errno == EINTR); - - dup2(s, 0); - dup2(s, 1); - dup2(s, 2); - for (s = getdtablesize() - 1; s >= 3; s--) - close(s); + setsid(); + dup2(sp[1], 0); + dup2(sp[1], 1); + dup2(sp[1], 2); + for (c = getdtablesize() - 1; c >= 3; c--) + close(c); i = 0; bptr = g_strdup(ex); /* No need to free() this */ @@ -163,23 +183,14 @@ fork_exec(struct socket *so, const char *ex) exit(1); default: - qemu_add_child_watch(pid); - /* - * XXX this could block us... - * XXX Should set a timer here, and if accept() doesn't - * return after X seconds, declare it a failure - * The only reason this will block forever is if socket() - * of connect() fail in the child process - */ - do { - so->s = accept(s, (struct sockaddr *)&addr, &addrlen); - } while (so->s < 0 && errno == EINTR); - closesocket(s); - socket_set_fast_reuse(so->s); - opt = 1; - qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int)); - qemu_set_nonblock(so->s); - return 1; + so->s = sp[0]; + closesocket(sp[1]); + qemu_add_child_watch(pid); + socket_set_fast_reuse(so->s); + opt = 1; + qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int)); + qemu_set_nonblock(so->s); + return 1; } } #endif -- 2.19.1.708.g4ede3d42df