[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH for-3.2 10/13] slirp: improve subprocess socket crea
From: |
Marc-André Lureau |
Subject: |
[Qemu-devel] [PATCH for-3.2 10/13] slirp: improve subprocess socket creation |
Date: |
Sat, 10 Nov 2018 17:45:45 +0400 |
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 <address@hidden>
---
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
- [Qemu-devel] [PATCH for-3.2 06/13] slirp: rename /extra/chardev, (continued)
- [Qemu-devel] [PATCH for-3.2 06/13] slirp: rename /extra/chardev, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 05/13] slirp: remove unused EMU_RSH, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 08/13] slirp: remove Monitor dependency, return a string for info, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 04/13] slirp: use a dedicated field for chardev pointer, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 09/13] slirp: fix slirp_add_exec() leaks, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 10/13] slirp: improve subprocess socket creation,
Marc-André Lureau <=
- [Qemu-devel] [PATCH for-3.2 11/13] slirp: replace the poor-man string split with g_strsplit(), Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 12/13] glib-compat: add g_spawn_async_with_fds() fallback, Marc-André Lureau, 2018/11/10
- [Qemu-devel] [PATCH for-3.2 13/13] slirp: simplify fork_exec(), Marc-André Lureau, 2018/11/10
- Re: [Qemu-devel] [PATCH for-3.2 00/13] slirp: cleanups, no-reply, 2018/11/10
- Re: [Qemu-devel] [PATCH for-3.2 00/13] slirp: cleanups, no-reply, 2018/11/10