* [Qemu-devel] [PATCH v2] Don't leak file descriptors
@ 2009-11-16 15:47 Kevin Wolf
2009-12-01 15:55 ` Alexander Graf
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2009-11-16 15:47 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf
We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
descriptors that don't need to be passed to children to stop this misbehaviour.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/raw-posix.c | 2 +-
gdbstub.c | 6 +++
kvm-all.c | 2 +-
migration-tcp.c | 6 +-
migration-unix.c | 6 +-
net.c | 8 ++--
osdep.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++-
posix-aio-compat.c | 2 +-
qemu-char.c | 8 ++--
qemu-common.h | 7 +++
qemu-sockets.c | 10 ++--
qemu_socket.h | 2 +
slirp/misc.c | 4 +-
slirp/slirp.h | 4 ++
slirp/socket.c | 2 +-
slirp/tcp_subr.c | 2 +-
slirp/udp.c | 4 +-
vl.c | 11 +++--
vnc.c | 2 +-
19 files changed, 158 insertions(+), 34 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index f558976..0ee8ff7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -153,7 +153,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
s->open_flags |= O_DSYNC;
s->fd = -1;
- fd = open(filename, s->open_flags, 0644);
+ fd = qemu_open(filename, s->open_flags, 0644);
if (fd < 0) {
ret = -errno;
if (ret == -EROFS)
diff --git a/gdbstub.c b/gdbstub.c
index 055093f..5a4f7d4 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2356,6 +2356,9 @@ static void gdb_accept(void)
perror("accept");
return;
} else if (fd >= 0) {
+#ifndef _WIN32
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
break;
}
}
@@ -2385,6 +2388,9 @@ static int gdbserver_open(int port)
perror("socket");
return -1;
}
+#ifndef _WIN32
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
/* allow fast reuse */
val = 1;
diff --git a/kvm-all.c b/kvm-all.c
index 1916ec6..fe6220c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -414,7 +414,7 @@ int kvm_init(int smp_cpus)
s->slots[i].slot = i;
s->vmfd = -1;
- s->fd = open("/dev/kvm", O_RDWR);
+ s->fd = qemu_open("/dev/kvm", O_RDWR);
if (s->fd == -1) {
fprintf(stderr, "Could not access KVM kernel module: %m\n");
ret = -errno;
diff --git a/migration-tcp.c b/migration-tcp.c
index 9ed92b4..dc8772c 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -99,7 +99,7 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
s->state = MIG_STATE_ACTIVE;
s->mon_resume = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->fd = socket(PF_INET, SOCK_STREAM, 0);
+ s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
qemu_free(s);
return NULL;
@@ -139,7 +139,7 @@ static void tcp_accept_incoming_migration(void *opaque)
int c, ret;
do {
- c = accept(s, (struct sockaddr *)&addr, &addrlen);
+ c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
} while (c == -1 && socket_error() == EINTR);
dprintf("accepted migration\n");
@@ -186,7 +186,7 @@ int tcp_start_incoming_migration(const char *host_port)
return -EINVAL;
}
- s = socket(PF_INET, SOCK_STREAM, 0);
+ s = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s == -1)
return -socket_error();
diff --git a/migration-unix.c b/migration-unix.c
index a26587a..d3de7ae 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -98,7 +98,7 @@ MigrationState *unix_start_outgoing_migration(const char *path,
s->state = MIG_STATE_ACTIVE;
s->mon_resume = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->fd = socket(PF_UNIX, SOCK_STREAM, 0);
+ s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (s->fd < 0) {
dprintf("Unable to open socket");
goto err_after_alloc;
@@ -143,7 +143,7 @@ static void unix_accept_incoming_migration(void *opaque)
int c, ret;
do {
- c = accept(s, (struct sockaddr *)&addr, &addrlen);
+ c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
} while (c == -1 && socket_error() == EINTR);
dprintf("accepted migration\n");
@@ -184,7 +184,7 @@ int unix_start_incoming_migration(const char *path)
dprintf("Attempting to start an incoming migration\n");
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
return -EINVAL;
diff --git a/net.c b/net.c
index 9ea66e3..cff6efd 100644
--- a/net.c
+++ b/net.c
@@ -1515,7 +1515,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
return -1;
}
- fd = socket(PF_INET, SOCK_DGRAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
if (fd < 0) {
perror("socket(PF_INET, SOCK_DGRAM)");
return -1;
@@ -1693,7 +1693,7 @@ static void net_socket_accept(void *opaque)
for(;;) {
len = sizeof(saddr);
- fd = accept(s->fd, (struct sockaddr *)&saddr, &len);
+ fd = qemu_accept(s->fd, (struct sockaddr *)&saddr, &len);
if (fd < 0 && errno != EINTR) {
return;
} else if (fd >= 0) {
@@ -1724,7 +1724,7 @@ static int net_socket_listen_init(VLANState *vlan,
s = qemu_mallocz(sizeof(NetSocketListenState));
- fd = socket(PF_INET, SOCK_STREAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
perror("socket");
return -1;
@@ -1765,7 +1765,7 @@ static int net_socket_connect_init(VLANState *vlan,
if (parse_host_port(&saddr, host_str) < 0)
return -1;
- fd = socket(PF_INET, SOCK_STREAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
perror("socket");
return -1;
diff --git a/osdep.c b/osdep.c
index fd8bbd7..039065e 100644
--- a/osdep.c
+++ b/osdep.c
@@ -121,7 +121,7 @@ int qemu_create_pidfile(const char *filename)
#ifndef _WIN32
int fd;
- fd = open(filename, O_RDWR | O_CREAT, 0600);
+ fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
if (fd == -1)
return -1;
@@ -201,11 +201,113 @@ int inet_aton(const char *cp, struct in_addr *ia)
ia->s_addr = addr;
return 1;
}
+
+void qemu_set_cloexec(int fd)
+{
+}
+
#else
+
void socket_set_nonblock(int fd)
{
int f;
f = fcntl(fd, F_GETFL);
fcntl(fd, F_SETFL, f | O_NONBLOCK);
}
+
+void qemu_set_cloexec(int fd)
+{
+ int f;
+ f = fcntl(fd, F_GETFD);
+ fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+}
+
+#endif
+
+/*
+ * Opens a file with FD_CLOEXEC set
+ */
+int qemu_open(const char *name, int flags, ...)
+{
+ int ret;
+ int mode = 0;
+
+ if (flags & O_CREAT) {
+ va_list ap;
+
+ va_start(ap, flags);
+ mode = va_arg(ap, int);
+ va_end(ap);
+ }
+
+#ifdef O_CLOEXEC
+ ret = open(name, flags | O_CLOEXEC, mode);
+#else
+ ret = open(name, flags, mode);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
#endif
+
+ return ret;
+}
+
+#ifndef _WIN32
+/*
+ * Creates a pipe with FD_CLOEXEC set on both file descriptors
+ */
+int qemu_pipe(int pipefd[2])
+{
+ int ret;
+
+#ifdef O_CLOEXEC
+ ret = pipe2(pipefd, O_CLOEXEC);
+#else
+ ret = pipe(pipefd);
+ if (ret == 0) {
+ qemu_set_cloexec(pipefd[0]);
+ qemu_set_cloexec(pipefd[1]);
+ }
+#endif
+
+ return ret;
+}
+#endif
+
+/*
+ * 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);
+#else
+ ret = socket(domain, type, protocol);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+
+ return ret;
+}
+
+/*
+ * Accept a connection and set FD_CLOEXEC
+ */
+int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
+{
+ int ret;
+
+#ifdef SOCK_CLOEXEC
+ ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
+#else
+ ret = accept(s, addr, addrlen);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+
+ return ret;
+}
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7f391c9..ac247e1 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -625,7 +625,7 @@ int paio_init(void)
sigaction(SIGUSR2, &act, NULL);
s->first_aio = NULL;
- if (pipe(fds) == -1) {
+ if (qemu_pipe(fds) == -1) {
fprintf(stderr, "failed to create pipe\n");
return -1;
}
diff --git a/qemu-char.c b/qemu-char.c
index 40bd7e8..8b5a8bb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -626,7 +626,7 @@ static CharDriverState *qemu_chr_open_file_out(QemuOpts *opts)
{
int fd_out;
- TFR(fd_out = open(qemu_opt_get(opts, "path"),
+ TFR(fd_out = qemu_open(qemu_opt_get(opts, "path"),
O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666));
if (fd_out < 0)
return NULL;
@@ -646,8 +646,8 @@ static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
snprintf(filename_in, 256, "%s.in", filename);
snprintf(filename_out, 256, "%s.out", filename);
- TFR(fd_in = open(filename_in, O_RDWR | O_BINARY));
- TFR(fd_out = open(filename_out, O_RDWR | O_BINARY));
+ TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
+ TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
if (fd_in < 0 || fd_out < 0) {
if (fd_in >= 0)
close(fd_in);
@@ -2106,7 +2106,7 @@ static void tcp_chr_accept(void *opaque)
len = sizeof(saddr);
addr = (struct sockaddr *)&saddr;
}
- fd = accept(s->listen_fd, addr, &len);
+ fd = qemu_accept(s->listen_fd, addr, &len);
if (fd < 0 && errno != EINTR) {
return;
} else if (fd >= 0) {
diff --git a/qemu-common.h b/qemu-common.h
index b779cfe..6975a29 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -159,6 +159,13 @@ void *get_mmap_addr(unsigned long size);
void qemu_mutex_lock_iothread(void);
void qemu_mutex_unlock_iothread(void);
+int qemu_open(const char *name, int flags, ...);
+void qemu_set_cloexec(int fd);
+
+#ifndef _WIN32
+int qemu_pipe(int pipefd[2]);
+#endif
+
/* Error handling. */
void QEMU_NORETURN hw_error(const char *fmt, ...)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 8801453..8850516 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -160,7 +160,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
uaddr,INET6_ADDRSTRLEN,uport,32,
NI_NUMERICHOST | NI_NUMERICSERV);
- slisten = socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+ slisten = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
if (slisten < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family), strerror(errno));
@@ -258,7 +258,7 @@ int inet_connect_opts(QemuOpts *opts)
fprintf(stderr,"%s: getnameinfo: oops\n", __FUNCTION__);
continue;
}
- sock = socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+ sock = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
if (sock < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family), strerror(errno));
@@ -351,7 +351,7 @@ int inet_dgram_opts(QemuOpts *opts)
}
/* create socket */
- sock = socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
+ sock = qemu_socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
if (sock < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(peer->ai_family), strerror(errno));
@@ -505,7 +505,7 @@ int unix_listen_opts(QemuOpts *opts)
const char *path = qemu_opt_get(opts, "path");
int sock, fd;
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(unix)");
return -1;
@@ -560,7 +560,7 @@ int unix_connect_opts(QemuOpts *opts)
return -1;
}
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(unix)");
return -1;
diff --git a/qemu_socket.h b/qemu_socket.h
index c253b32..86bdbf5 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -32,6 +32,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
#include "qemu-option.h"
/* misc helpers */
+int qemu_socket(int domain, int type, int protocol);
+int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
void socket_set_nonblock(int fd);
int send_all(int fd, const void *buf, int len1);
diff --git a/slirp/misc.c b/slirp/misc.c
index e9f08fd..c76ad8f 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -132,7 +132,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 = socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
+ if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
listen(s, 1) < 0) {
lprint("Error: inet socket: %s\n", strerror(errno));
@@ -165,7 +165,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 = socket(AF_INET, SOCK_STREAM, 0);
+ s = qemu_socket(AF_INET, SOCK_STREAM, 0);
addr.sin_addr = loopback_addr;
do {
ret = connect(s, (struct sockaddr *)&addr, addrlen);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 9ef57ea..98a2644 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -197,6 +197,10 @@ int inet_aton(const char *cp, struct in_addr *ia);
#include "bootp.h"
#include "tftp.h"
+/* osdep.c */
+int qemu_socket(int domain, int type, int protocol);
+
+
struct Slirp {
QTAILQ_ENTRY(Slirp) entry;
diff --git a/slirp/socket.c b/slirp/socket.c
index 207109c..cf6e6a9 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -626,7 +626,7 @@ tcp_listen(Slirp *slirp, u_int32_t haddr, u_int hport, u_int32_t laddr,
addr.sin_addr.s_addr = haddr;
addr.sin_port = hport;
- if (((s = socket(AF_INET,SOCK_STREAM,0)) < 0) ||
+ if (((s = qemu_socket(AF_INET,SOCK_STREAM,0)) < 0) ||
(setsockopt(s,SOL_SOCKET,SO_REUSEADDR,(char *)&opt,sizeof(int)) < 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 0417345..7851307 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -325,7 +325,7 @@ int tcp_fconnect(struct socket *so)
DEBUG_CALL("tcp_fconnect");
DEBUG_ARG("so = %lx", (long )so);
- if( (ret=so->s=socket(AF_INET,SOCK_STREAM,0)) >= 0) {
+ if( (ret = so->s = qemu_socket(AF_INET,SOCK_STREAM,0)) >= 0) {
int opt, s=so->s;
struct sockaddr_in addr;
diff --git a/slirp/udp.c b/slirp/udp.c
index a88b645..d6c39b9 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -302,7 +302,7 @@ int udp_output(struct socket *so, struct mbuf *m,
int
udp_attach(struct socket *so)
{
- if((so->s = socket(AF_INET,SOCK_DGRAM,0)) != -1) {
+ if((so->s = qemu_socket(AF_INET,SOCK_DGRAM,0)) != -1) {
so->so_expire = curtime + SO_EXPIRE;
insque(so, &so->slirp->udb);
}
@@ -350,7 +350,7 @@ udp_listen(Slirp *slirp, u_int32_t haddr, u_int hport, u_int32_t laddr,
if (!so) {
return NULL;
}
- so->s = socket(AF_INET,SOCK_DGRAM,0);
+ so->s = qemu_socket(AF_INET,SOCK_DGRAM,0);
so->so_expire = curtime + SO_EXPIRE;
insque(so, &slirp->udb);
diff --git a/vl.c b/vl.c
index fff8e8d..94d82e0 100644
--- a/vl.c
+++ b/vl.c
@@ -1242,7 +1242,7 @@ static int hpet_start_timer(struct qemu_alarm_timer *t)
struct hpet_info info;
int r, fd;
- fd = open("/dev/hpet", O_RDONLY);
+ fd = qemu_open("/dev/hpet", O_RDONLY);
if (fd < 0)
return -1;
@@ -1291,7 +1291,7 @@ static int rtc_start_timer(struct qemu_alarm_timer *t)
int rtc_fd;
unsigned long current_rtc_freq = 0;
- TFR(rtc_fd = open("/dev/rtc", O_RDONLY));
+ TFR(rtc_fd = qemu_open("/dev/rtc", O_RDONLY));
if (rtc_fd < 0)
return -1;
ioctl(rtc_fd, RTC_IRQP_READ, ¤t_rtc_freq);
@@ -3301,7 +3301,7 @@ static int qemu_event_init(void)
int err;
int fds[2];
- err = pipe(fds);
+ err = qemu_pipe(fds);
if (err == -1)
return -errno;
@@ -5461,6 +5461,9 @@ int main(int argc, char **argv, char **envp)
} else if (pid < 0)
exit(1);
+ close(fds[0]);
+ qemu_set_cloexec(fds[1]);
+
setsid();
pid = fork();
@@ -5861,7 +5864,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
chdir("/");
- TFR(fd = open("/dev/null", O_RDWR));
+ TFR(fd = qemu_open("/dev/null", O_RDWR));
if (fd == -1)
exit(1);
}
diff --git a/vnc.c b/vnc.c
index 2bb8024..32c4678 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2247,7 +2247,7 @@ static void vnc_listen_read(void *opaque)
/* Catch-up */
vga_hw_update();
- int csock = accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
+ int csock = qemu_accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
if (csock != -1) {
vnc_connect(vs, csock);
}
--
1.6.2.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors
2009-11-16 15:47 [Qemu-devel] " Kevin Wolf
@ 2009-12-01 15:55 ` Alexander Graf
2009-12-02 10:34 ` Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Graf @ 2009-12-01 15:55 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Anthony Liguori, qemu-devel
Kevin Wolf wrote:
> We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
> descriptors that don't need to be passed to children to stop this misbehaviour.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/raw-posix.c | 2 +-
> gdbstub.c | 6 +++
> kvm-all.c | 2 +-
> migration-tcp.c | 6 +-
> migration-unix.c | 6 +-
> net.c | 8 ++--
> osdep.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> posix-aio-compat.c | 2 +-
> qemu-char.c | 8 ++--
> qemu-common.h | 7 +++
> qemu-sockets.c | 10 ++--
> qemu_socket.h | 2 +
> slirp/misc.c | 4 +-
> slirp/slirp.h | 4 ++
> slirp/socket.c | 2 +-
> slirp/tcp_subr.c | 2 +-
> slirp/udp.c | 4 +-
> vl.c | 11 +++--
> vnc.c | 2 +-
> 19 files changed, 158 insertions(+), 34 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index f558976..0ee8ff7 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -153,7 +153,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
> s->open_flags |= O_DSYNC;
>
> s->fd = -1;
> - fd = open(filename, s->open_flags, 0644);
> + fd = qemu_open(filename, s->open_flags, 0644);
> if (fd < 0) {
> ret = -errno;
> if (ret == -EROFS)
> diff --git a/gdbstub.c b/gdbstub.c
> index 055093f..5a4f7d4 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2356,6 +2356,9 @@ static void gdb_accept(void)
> perror("accept");
> return;
> } else if (fd >= 0) {
> +#ifndef _WIN32
> + fcntl(fd, F_SETFD, FD_CLOEXEC);
> +#endif
> break;
> }
> }
> @@ -2385,6 +2388,9 @@ static int gdbserver_open(int port)
> perror("socket");
> return -1;
> }
> +#ifndef _WIN32
> + fcntl(fd, F_SETFD, FD_CLOEXEC);
> +#endif
>
> /* allow fast reuse */
> val = 1;
> diff --git a/kvm-all.c b/kvm-all.c
> index 1916ec6..fe6220c 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -414,7 +414,7 @@ int kvm_init(int smp_cpus)
> s->slots[i].slot = i;
>
> s->vmfd = -1;
> - s->fd = open("/dev/kvm", O_RDWR);
> + s->fd = qemu_open("/dev/kvm", O_RDWR);
> if (s->fd == -1) {
> fprintf(stderr, "Could not access KVM kernel module: %m\n");
> ret = -errno;
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 9ed92b4..dc8772c 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -99,7 +99,7 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
> s->state = MIG_STATE_ACTIVE;
> s->mon_resume = NULL;
> s->bandwidth_limit = bandwidth_limit;
> - s->fd = socket(PF_INET, SOCK_STREAM, 0);
> + s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (s->fd == -1) {
> qemu_free(s);
> return NULL;
> @@ -139,7 +139,7 @@ static void tcp_accept_incoming_migration(void *opaque)
> int c, ret;
>
> do {
> - c = accept(s, (struct sockaddr *)&addr, &addrlen);
> + c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> } while (c == -1 && socket_error() == EINTR);
>
> dprintf("accepted migration\n");
> @@ -186,7 +186,7 @@ int tcp_start_incoming_migration(const char *host_port)
> return -EINVAL;
> }
>
> - s = socket(PF_INET, SOCK_STREAM, 0);
> + s = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (s == -1)
> return -socket_error();
>
> diff --git a/migration-unix.c b/migration-unix.c
> index a26587a..d3de7ae 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -98,7 +98,7 @@ MigrationState *unix_start_outgoing_migration(const char *path,
> s->state = MIG_STATE_ACTIVE;
> s->mon_resume = NULL;
> s->bandwidth_limit = bandwidth_limit;
> - s->fd = socket(PF_UNIX, SOCK_STREAM, 0);
> + s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> if (s->fd < 0) {
> dprintf("Unable to open socket");
> goto err_after_alloc;
> @@ -143,7 +143,7 @@ static void unix_accept_incoming_migration(void *opaque)
> int c, ret;
>
> do {
> - c = accept(s, (struct sockaddr *)&addr, &addrlen);
> + c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> } while (c == -1 && socket_error() == EINTR);
>
> dprintf("accepted migration\n");
> @@ -184,7 +184,7 @@ int unix_start_incoming_migration(const char *path)
>
> dprintf("Attempting to start an incoming migration\n");
>
> - sock = socket(PF_UNIX, SOCK_STREAM, 0);
> + sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> if (sock < 0) {
> fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
> return -EINVAL;
> diff --git a/net.c b/net.c
> index 9ea66e3..cff6efd 100644
> --- a/net.c
> +++ b/net.c
> @@ -1515,7 +1515,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
> return -1;
>
> }
> - fd = socket(PF_INET, SOCK_DGRAM, 0);
> + fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
> if (fd < 0) {
> perror("socket(PF_INET, SOCK_DGRAM)");
> return -1;
> @@ -1693,7 +1693,7 @@ static void net_socket_accept(void *opaque)
>
> for(;;) {
> len = sizeof(saddr);
> - fd = accept(s->fd, (struct sockaddr *)&saddr, &len);
> + fd = qemu_accept(s->fd, (struct sockaddr *)&saddr, &len);
> if (fd < 0 && errno != EINTR) {
> return;
> } else if (fd >= 0) {
> @@ -1724,7 +1724,7 @@ static int net_socket_listen_init(VLANState *vlan,
>
> s = qemu_mallocz(sizeof(NetSocketListenState));
>
> - fd = socket(PF_INET, SOCK_STREAM, 0);
> + fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (fd < 0) {
> perror("socket");
> return -1;
> @@ -1765,7 +1765,7 @@ static int net_socket_connect_init(VLANState *vlan,
> if (parse_host_port(&saddr, host_str) < 0)
> return -1;
>
> - fd = socket(PF_INET, SOCK_STREAM, 0);
> + fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> if (fd < 0) {
> perror("socket");
> return -1;
> diff --git a/osdep.c b/osdep.c
> index fd8bbd7..039065e 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -121,7 +121,7 @@ int qemu_create_pidfile(const char *filename)
> #ifndef _WIN32
> int fd;
>
> - fd = open(filename, O_RDWR | O_CREAT, 0600);
> + fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
> if (fd == -1)
> return -1;
>
> @@ -201,11 +201,113 @@ int inet_aton(const char *cp, struct in_addr *ia)
> ia->s_addr = addr;
> return 1;
> }
> +
> +void qemu_set_cloexec(int fd)
> +{
> +}
> +
> #else
> +
> void socket_set_nonblock(int fd)
> {
> int f;
> f = fcntl(fd, F_GETFL);
> fcntl(fd, F_SETFL, f | O_NONBLOCK);
> }
> +
> +void qemu_set_cloexec(int fd)
> +{
> + int f;
> + f = fcntl(fd, F_GETFD);
> + fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +}
> +
> +#endif
> +
> +/*
> + * Opens a file with FD_CLOEXEC set
> + */
> +int qemu_open(const char *name, int flags, ...)
> +{
> + int ret;
> + int mode = 0;
> +
> + if (flags & O_CREAT) {
> + va_list ap;
> +
> + va_start(ap, flags);
> + mode = va_arg(ap, int);
> + va_end(ap);
> + }
> +
> +#ifdef O_CLOEXEC
> + ret = open(name, flags | O_CLOEXEC, mode);
> +#else
> + ret = open(name, flags, mode);
> + if (ret >= 0) {
> + qemu_set_cloexec(ret);
> + }
> #endif
> +
> + return ret;
> +}
> +
> +#ifndef _WIN32
> +/*
> + * Creates a pipe with FD_CLOEXEC set on both file descriptors
> + */
> +int qemu_pipe(int pipefd[2])
> +{
> + int ret;
> +
> +#ifdef O_CLOEXEC
> + ret = pipe2(pipefd, O_CLOEXEC);
> +#else
> + ret = pipe(pipefd);
> + if (ret == 0) {
> + qemu_set_cloexec(pipefd[0]);
> + qemu_set_cloexec(pipefd[1]);
> + }
> +#endif
> +
> + return ret;
> +}
> +#endif
> +
> +/*
> + * 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);
> +#else
> + ret = socket(domain, type, protocol);
> + if (ret >= 0) {
> + qemu_set_cloexec(ret);
> + }
> +#endif
> +
> + return ret;
> +}
> +
> +/*
> + * Accept a connection and set FD_CLOEXEC
> + */
> +int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
> +{
> + int ret;
> +
> +#ifdef SOCK_CLOEXEC
> + ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
>
On Anthony's staging tree:
cc1: warnings being treated as errors
osdep.c: In function ‘qemu_accept’:
osdep.c:304: error: implicit declaration of function ‘accept4’
Alex
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors
2009-12-01 15:55 ` Alexander Graf
@ 2009-12-02 10:34 ` Kevin Wolf
0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2009-12-02 10:34 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
Am 01.12.2009 16:55, schrieb Alexander Graf:
> Kevin Wolf wrote:
>> We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
>> descriptors that don't need to be passed to children to stop this misbehaviour.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>
> On Anthony's staging tree:
>
> cc1: warnings being treated as errors
> osdep.c: In function ‘qemu_accept’:
> osdep.c:304: error: implicit declaration of function ‘accept4’
Does this one on top work for you?
Kevin
[-- Attachment #2: fix_accept4.patch --]
[-- Type: text/plain, Size: 1315 bytes --]
diff --git a/configure b/configure
index dca5a43..0a1e347 100755
--- a/configure
+++ b/configure
@@ -1550,6 +1550,23 @@ if compile_prog "" "" ; then
pipe2=yes
fi
+# check if accept4 is there
+accept4=no
+cat > $TMPC << EOF
+#define _GNU_SOURCE
+#include <sys/socket.h>
+#include <stddef.h>
+
+int main(void)
+{
+ accept4(0, NULL, NULL, SOCK_CLOEXEC);
+ return 0;
+}
+EOF
+if compile_prog "" "" ; then
+ accept4=yes
+fi
+
# check if tee/splice is there. vmsplice was added same time.
splice=no
cat > $TMPC << EOF
@@ -2013,6 +2030,9 @@ fi
if test "$pipe2" = "yes" ; then
echo "CONFIG_PIPE2=y" >> $config_host_mak
fi
+if test "$accept4" = "yes" ; then
+ echo "CONFIG_ACCEPT4=y" >> $config_host_mak
+fi
if test "$splice" = "yes" ; then
echo "CONFIG_SPLICE=y" >> $config_host_mak
fi
diff --git a/osdep.c b/osdep.c
index 039065e..7509c5b 100644
--- a/osdep.c
+++ b/osdep.c
@@ -260,7 +260,7 @@ int qemu_pipe(int pipefd[2])
{
int ret;
-#ifdef O_CLOEXEC
+#ifdef CONFIG_PIPE2
ret = pipe2(pipefd, O_CLOEXEC);
#else
ret = pipe(pipefd);
@@ -300,7 +300,7 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
{
int ret;
-#ifdef SOCK_CLOEXEC
+#ifdef CONFIG_ACCEPT4
ret = accept4(s, addr, addrlen, SOCK_CLOEXEC);
#else
ret = accept(s, addr, addrlen);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2] Don't leak file descriptors
@ 2009-12-02 11:24 Kevin Wolf
2009-12-02 11:26 ` [Qemu-devel] " Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: Kevin Wolf @ 2009-12-02 11:24 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf
We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
descriptors that don't need to be passed to children to stop this misbehaviour.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
v2:
- The existence of SOCK_CLOEXEC doesn't mean that accept4 exists. Added a check
for it in configure.
block/raw-posix.c | 2 +-
configure | 20 ++++++++++
gdbstub.c | 6 +++
kvm-all.c | 2 +-
migration-tcp.c | 6 +-
migration-unix.c | 6 +-
net.c | 8 ++--
osdep.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++-
posix-aio-compat.c | 2 +-
qemu-char.c | 8 ++--
qemu-common.h | 7 +++
qemu-sockets.c | 10 ++--
qemu_socket.h | 2 +
slirp/misc.c | 4 +-
slirp/slirp.h | 4 ++
slirp/socket.c | 2 +-
slirp/tcp_subr.c | 2 +-
slirp/udp.c | 4 +-
vl.c | 11 +++--
vnc.c | 2 +-
20 files changed, 178 insertions(+), 34 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 266d841..0359e43 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -153,7 +153,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
s->open_flags |= O_DSYNC;
s->fd = -1;
- fd = open(filename, s->open_flags, 0644);
+ fd = qemu_open(filename, s->open_flags, 0644);
if (fd < 0) {
ret = -errno;
if (ret == -EROFS)
diff --git a/configure b/configure
index dca5a43..0a1e347 100755
--- a/configure
+++ b/configure
@@ -1550,6 +1550,23 @@ if compile_prog "" "" ; then
pipe2=yes
fi
+# check if accept4 is there
+accept4=no
+cat > $TMPC << EOF
+#define _GNU_SOURCE
+#include <sys/socket.h>
+#include <stddef.h>
+
+int main(void)
+{
+ accept4(0, NULL, NULL, SOCK_CLOEXEC);
+ return 0;
+}
+EOF
+if compile_prog "" "" ; then
+ accept4=yes
+fi
+
# check if tee/splice is there. vmsplice was added same time.
splice=no
cat > $TMPC << EOF
@@ -2013,6 +2030,9 @@ fi
if test "$pipe2" = "yes" ; then
echo "CONFIG_PIPE2=y" >> $config_host_mak
fi
+if test "$accept4" = "yes" ; then
+ echo "CONFIG_ACCEPT4=y" >> $config_host_mak
+fi
if test "$splice" = "yes" ; then
echo "CONFIG_SPLICE=y" >> $config_host_mak
fi
diff --git a/gdbstub.c b/gdbstub.c
index 055093f..5a4f7d4 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2356,6 +2356,9 @@ static void gdb_accept(void)
perror("accept");
return;
} else if (fd >= 0) {
+#ifndef _WIN32
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
break;
}
}
@@ -2385,6 +2388,9 @@ static int gdbserver_open(int port)
perror("socket");
return -1;
}
+#ifndef _WIN32
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
/* allow fast reuse */
val = 1;
diff --git a/kvm-all.c b/kvm-all.c
index b605caa..b8ffe54 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -416,7 +416,7 @@ int kvm_init(int smp_cpus)
s->slots[i].slot = i;
s->vmfd = -1;
- s->fd = open("/dev/kvm", O_RDWR);
+ s->fd = qemu_open("/dev/kvm", O_RDWR);
if (s->fd == -1) {
fprintf(stderr, "Could not access KVM kernel module: %m\n");
ret = -errno;
diff --git a/migration-tcp.c b/migration-tcp.c
index efa7c74..4bd19ed 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -104,7 +104,7 @@ MigrationState *tcp_start_outgoing_migration(const char *host_port,
s->state = MIG_STATE_ACTIVE;
s->mon_resume = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->fd = socket(PF_INET, SOCK_STREAM, 0);
+ s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s->fd == -1) {
qemu_free(s);
return NULL;
@@ -144,7 +144,7 @@ static void tcp_accept_incoming_migration(void *opaque)
int c, ret;
do {
- c = accept(s, (struct sockaddr *)&addr, &addrlen);
+ c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
} while (c == -1 && socket_error() == EINTR);
dprintf("accepted migration\n");
@@ -191,7 +191,7 @@ int tcp_start_incoming_migration(const char *host_port)
return -EINVAL;
}
- s = socket(PF_INET, SOCK_STREAM, 0);
+ s = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (s == -1)
return -socket_error();
diff --git a/migration-unix.c b/migration-unix.c
index 25cd6d3..4d03f6a 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -103,7 +103,7 @@ MigrationState *unix_start_outgoing_migration(const char *path,
s->state = MIG_STATE_ACTIVE;
s->mon_resume = NULL;
s->bandwidth_limit = bandwidth_limit;
- s->fd = socket(PF_UNIX, SOCK_STREAM, 0);
+ s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (s->fd < 0) {
dprintf("Unable to open socket");
goto err_after_alloc;
@@ -148,7 +148,7 @@ static void unix_accept_incoming_migration(void *opaque)
int c, ret;
do {
- c = accept(s, (struct sockaddr *)&addr, &addrlen);
+ c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
} while (c == -1 && socket_error() == EINTR);
dprintf("accepted migration\n");
@@ -189,7 +189,7 @@ int unix_start_incoming_migration(const char *path)
dprintf("Attempting to start an incoming migration\n");
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
fprintf(stderr, "Could not open unix socket: %s\n", strerror(errno));
return -EINVAL;
diff --git a/net.c b/net.c
index 7c44c1a..50a6db8 100644
--- a/net.c
+++ b/net.c
@@ -1512,7 +1512,7 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
return -1;
}
- fd = socket(PF_INET, SOCK_DGRAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
if (fd < 0) {
perror("socket(PF_INET, SOCK_DGRAM)");
return -1;
@@ -1690,7 +1690,7 @@ static void net_socket_accept(void *opaque)
for(;;) {
len = sizeof(saddr);
- fd = accept(s->fd, (struct sockaddr *)&saddr, &len);
+ fd = qemu_accept(s->fd, (struct sockaddr *)&saddr, &len);
if (fd < 0 && errno != EINTR) {
return;
} else if (fd >= 0) {
@@ -1721,7 +1721,7 @@ static int net_socket_listen_init(VLANState *vlan,
s = qemu_mallocz(sizeof(NetSocketListenState));
- fd = socket(PF_INET, SOCK_STREAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
perror("socket");
return -1;
@@ -1762,7 +1762,7 @@ static int net_socket_connect_init(VLANState *vlan,
if (parse_host_port(&saddr, host_str) < 0)
return -1;
- fd = socket(PF_INET, SOCK_STREAM, 0);
+ fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
perror("socket");
return -1;
diff --git a/osdep.c b/osdep.c
index fd8bbd7..7509c5b 100644
--- a/osdep.c
+++ b/osdep.c
@@ -121,7 +121,7 @@ int qemu_create_pidfile(const char *filename)
#ifndef _WIN32
int fd;
- fd = open(filename, O_RDWR | O_CREAT, 0600);
+ fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
if (fd == -1)
return -1;
@@ -201,11 +201,113 @@ int inet_aton(const char *cp, struct in_addr *ia)
ia->s_addr = addr;
return 1;
}
+
+void qemu_set_cloexec(int fd)
+{
+}
+
#else
+
void socket_set_nonblock(int fd)
{
int f;
f = fcntl(fd, F_GETFL);
fcntl(fd, F_SETFL, f | O_NONBLOCK);
}
+
+void qemu_set_cloexec(int fd)
+{
+ int f;
+ f = fcntl(fd, F_GETFD);
+ fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+}
+
+#endif
+
+/*
+ * Opens a file with FD_CLOEXEC set
+ */
+int qemu_open(const char *name, int flags, ...)
+{
+ int ret;
+ int mode = 0;
+
+ if (flags & O_CREAT) {
+ va_list ap;
+
+ va_start(ap, flags);
+ mode = va_arg(ap, int);
+ va_end(ap);
+ }
+
+#ifdef O_CLOEXEC
+ ret = open(name, flags | O_CLOEXEC, mode);
+#else
+ ret = open(name, flags, mode);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
#endif
+
+ return ret;
+}
+
+#ifndef _WIN32
+/*
+ * Creates a pipe with FD_CLOEXEC set on both file descriptors
+ */
+int qemu_pipe(int pipefd[2])
+{
+ int ret;
+
+#ifdef CONFIG_PIPE2
+ ret = pipe2(pipefd, O_CLOEXEC);
+#else
+ ret = pipe(pipefd);
+ if (ret == 0) {
+ qemu_set_cloexec(pipefd[0]);
+ qemu_set_cloexec(pipefd[1]);
+ }
+#endif
+
+ return ret;
+}
+#endif
+
+/*
+ * 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);
+#else
+ ret = socket(domain, type, protocol);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+
+ 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);
+#else
+ ret = accept(s, addr, addrlen);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+
+ return ret;
+}
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7f391c9..ac247e1 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -625,7 +625,7 @@ int paio_init(void)
sigaction(SIGUSR2, &act, NULL);
s->first_aio = NULL;
- if (pipe(fds) == -1) {
+ if (qemu_pipe(fds) == -1) {
fprintf(stderr, "failed to create pipe\n");
return -1;
}
diff --git a/qemu-char.c b/qemu-char.c
index e202585..da5c15c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -620,7 +620,7 @@ static CharDriverState *qemu_chr_open_file_out(QemuOpts *opts)
{
int fd_out;
- TFR(fd_out = open(qemu_opt_get(opts, "path"),
+ TFR(fd_out = qemu_open(qemu_opt_get(opts, "path"),
O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666));
if (fd_out < 0)
return NULL;
@@ -640,8 +640,8 @@ static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
snprintf(filename_in, 256, "%s.in", filename);
snprintf(filename_out, 256, "%s.out", filename);
- TFR(fd_in = open(filename_in, O_RDWR | O_BINARY));
- TFR(fd_out = open(filename_out, O_RDWR | O_BINARY));
+ TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
+ TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
if (fd_in < 0 || fd_out < 0) {
if (fd_in >= 0)
close(fd_in);
@@ -2101,7 +2101,7 @@ static void tcp_chr_accept(void *opaque)
len = sizeof(saddr);
addr = (struct sockaddr *)&saddr;
}
- fd = accept(s->listen_fd, addr, &len);
+ fd = qemu_accept(s->listen_fd, addr, &len);
if (fd < 0 && errno != EINTR) {
return;
} else if (fd >= 0) {
diff --git a/qemu-common.h b/qemu-common.h
index 57af677..8630f8c 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -159,6 +159,13 @@ void *get_mmap_addr(unsigned long size);
void qemu_mutex_lock_iothread(void);
void qemu_mutex_unlock_iothread(void);
+int qemu_open(const char *name, int flags, ...);
+void qemu_set_cloexec(int fd);
+
+#ifndef _WIN32
+int qemu_pipe(int pipefd[2]);
+#endif
+
/* Error handling. */
void QEMU_NORETURN hw_error(const char *fmt, ...)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 8801453..8850516 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -160,7 +160,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset)
getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
uaddr,INET6_ADDRSTRLEN,uport,32,
NI_NUMERICHOST | NI_NUMERICSERV);
- slisten = socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+ slisten = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
if (slisten < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family), strerror(errno));
@@ -258,7 +258,7 @@ int inet_connect_opts(QemuOpts *opts)
fprintf(stderr,"%s: getnameinfo: oops\n", __FUNCTION__);
continue;
}
- sock = socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+ sock = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
if (sock < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family), strerror(errno));
@@ -351,7 +351,7 @@ int inet_dgram_opts(QemuOpts *opts)
}
/* create socket */
- sock = socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
+ sock = qemu_socket(peer->ai_family, peer->ai_socktype, peer->ai_protocol);
if (sock < 0) {
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(peer->ai_family), strerror(errno));
@@ -505,7 +505,7 @@ int unix_listen_opts(QemuOpts *opts)
const char *path = qemu_opt_get(opts, "path");
int sock, fd;
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(unix)");
return -1;
@@ -560,7 +560,7 @@ int unix_connect_opts(QemuOpts *opts)
return -1;
}
- sock = socket(PF_UNIX, SOCK_STREAM, 0);
+ sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(unix)");
return -1;
diff --git a/qemu_socket.h b/qemu_socket.h
index c253b32..86bdbf5 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -32,6 +32,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
#include "qemu-option.h"
/* misc helpers */
+int qemu_socket(int domain, int type, int protocol);
+int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
void socket_set_nonblock(int fd);
int send_all(int fd, const void *buf, int len1);
diff --git a/slirp/misc.c b/slirp/misc.c
index e9f08fd..c76ad8f 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -132,7 +132,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 = socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
+ if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) < 0 ||
bind(s, (struct sockaddr *)&addr, addrlen) < 0 ||
listen(s, 1) < 0) {
lprint("Error: inet socket: %s\n", strerror(errno));
@@ -165,7 +165,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 = socket(AF_INET, SOCK_STREAM, 0);
+ s = qemu_socket(AF_INET, SOCK_STREAM, 0);
addr.sin_addr = loopback_addr;
do {
ret = connect(s, (struct sockaddr *)&addr, addrlen);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 9ef57ea..98a2644 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -197,6 +197,10 @@ int inet_aton(const char *cp, struct in_addr *ia);
#include "bootp.h"
#include "tftp.h"
+/* osdep.c */
+int qemu_socket(int domain, int type, int protocol);
+
+
struct Slirp {
QTAILQ_ENTRY(Slirp) entry;
diff --git a/slirp/socket.c b/slirp/socket.c
index 207109c..cf6e6a9 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -626,7 +626,7 @@ tcp_listen(Slirp *slirp, u_int32_t haddr, u_int hport, u_int32_t laddr,
addr.sin_addr.s_addr = haddr;
addr.sin_port = hport;
- if (((s = socket(AF_INET,SOCK_STREAM,0)) < 0) ||
+ if (((s = qemu_socket(AF_INET,SOCK_STREAM,0)) < 0) ||
(setsockopt(s,SOL_SOCKET,SO_REUSEADDR,(char *)&opt,sizeof(int)) < 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 0417345..7851307 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -325,7 +325,7 @@ int tcp_fconnect(struct socket *so)
DEBUG_CALL("tcp_fconnect");
DEBUG_ARG("so = %lx", (long )so);
- if( (ret=so->s=socket(AF_INET,SOCK_STREAM,0)) >= 0) {
+ if( (ret = so->s = qemu_socket(AF_INET,SOCK_STREAM,0)) >= 0) {
int opt, s=so->s;
struct sockaddr_in addr;
diff --git a/slirp/udp.c b/slirp/udp.c
index a88b645..d6c39b9 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -302,7 +302,7 @@ int udp_output(struct socket *so, struct mbuf *m,
int
udp_attach(struct socket *so)
{
- if((so->s = socket(AF_INET,SOCK_DGRAM,0)) != -1) {
+ if((so->s = qemu_socket(AF_INET,SOCK_DGRAM,0)) != -1) {
so->so_expire = curtime + SO_EXPIRE;
insque(so, &so->slirp->udb);
}
@@ -350,7 +350,7 @@ udp_listen(Slirp *slirp, u_int32_t haddr, u_int hport, u_int32_t laddr,
if (!so) {
return NULL;
}
- so->s = socket(AF_INET,SOCK_DGRAM,0);
+ so->s = qemu_socket(AF_INET,SOCK_DGRAM,0);
so->so_expire = curtime + SO_EXPIRE;
insque(so, &slirp->udb);
diff --git a/vl.c b/vl.c
index 44763af..3e8263d 100644
--- a/vl.c
+++ b/vl.c
@@ -1242,7 +1242,7 @@ static int hpet_start_timer(struct qemu_alarm_timer *t)
struct hpet_info info;
int r, fd;
- fd = open("/dev/hpet", O_RDONLY);
+ fd = qemu_open("/dev/hpet", O_RDONLY);
if (fd < 0)
return -1;
@@ -1291,7 +1291,7 @@ static int rtc_start_timer(struct qemu_alarm_timer *t)
int rtc_fd;
unsigned long current_rtc_freq = 0;
- TFR(rtc_fd = open("/dev/rtc", O_RDONLY));
+ TFR(rtc_fd = qemu_open("/dev/rtc", O_RDONLY));
if (rtc_fd < 0)
return -1;
ioctl(rtc_fd, RTC_IRQP_READ, ¤t_rtc_freq);
@@ -3299,7 +3299,7 @@ static int qemu_event_init(void)
int err;
int fds[2];
- err = pipe(fds);
+ err = qemu_pipe(fds);
if (err == -1)
return -errno;
@@ -5457,6 +5457,9 @@ int main(int argc, char **argv, char **envp)
} else if (pid < 0)
exit(1);
+ close(fds[0]);
+ qemu_set_cloexec(fds[1]);
+
setsid();
pid = fork();
@@ -5859,7 +5862,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
chdir("/");
- TFR(fd = open("/dev/null", O_RDWR));
+ TFR(fd = qemu_open("/dev/null", O_RDWR));
if (fd == -1)
exit(1);
}
diff --git a/vnc.c b/vnc.c
index 2bb8024..32c4678 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2247,7 +2247,7 @@ static void vnc_listen_read(void *opaque)
/* Catch-up */
vga_hw_update();
- int csock = accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
+ int csock = qemu_accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
if (csock != -1) {
vnc_connect(vs, csock);
}
--
1.6.2.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH v2] Don't leak file descriptors
2009-12-02 11:24 [Qemu-devel] [PATCH v2] Don't leak file descriptors Kevin Wolf
@ 2009-12-02 11:26 ` Kevin Wolf
2009-12-02 12:03 ` Scott Tsai
2009-12-02 14:04 ` Anthony Liguori
0 siblings, 2 replies; 8+ messages in thread
From: Kevin Wolf @ 2009-12-02 11:26 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
Am 02.12.2009 12:24, schrieb Kevin Wolf:
> We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
> descriptors that don't need to be passed to children to stop this misbehaviour.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> v2:
> - The existence of SOCK_CLOEXEC doesn't mean that accept4 exists. Added a check
> for it in configure.
v3 even. Anthony, I hope this doesn't confuse your scripts?
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2] Don't leak file descriptors
2009-12-02 11:26 ` [Qemu-devel] " Kevin Wolf
@ 2009-12-02 12:03 ` Scott Tsai
2009-12-02 12:58 ` Kevin Wolf
2009-12-02 14:04 ` Anthony Liguori
1 sibling, 1 reply; 8+ messages in thread
From: Scott Tsai @ 2009-12-02 12:03 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
On Wed, Dec 2, 2009 at 7:26 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> v3 even. Anthony, I hope this doesn't confuse your scripts?
Kevin, I see use of fopen, fdopen, popen, eventfd in qemu without the
equivalent of CLOEXEC set.
Do you want to handle those in this patch series as well?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2] Don't leak file descriptors
2009-12-02 12:03 ` Scott Tsai
@ 2009-12-02 12:58 ` Kevin Wolf
0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2009-12-02 12:58 UTC (permalink / raw)
To: Scott Tsai; +Cc: qemu-devel
Am 02.12.2009 13:03, schrieb Scott Tsai:
> On Wed, Dec 2, 2009 at 7:26 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> v3 even. Anthony, I hope this doesn't confuse your scripts?
>
> Kevin, I see use of fopen, fdopen, popen, eventfd in qemu without the
> equivalent of CLOEXEC set.
> Do you want to handle those in this patch series as well?
In the first instance, I'd like to see this one go in as it fixes the
common cases. Otherwise we could probably reiterate with new versions
for quite a while until really all files are covered.
It's well possible that I still missed some calls. I haven't seen any
file descriptors leaked any more in my test runs, but probably you just
need to use the right options. Feel free to follow up with an additional
patch.
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH v2] Don't leak file descriptors
2009-12-02 11:26 ` [Qemu-devel] " Kevin Wolf
2009-12-02 12:03 ` Scott Tsai
@ 2009-12-02 14:04 ` Anthony Liguori
1 sibling, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2009-12-02 14:04 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
Kevin Wolf wrote:
> Am 02.12.2009 12:24, schrieb Kevin Wolf:
>
>> We're leaking file descriptors to child processes. Set FD_CLOEXEC on file
>> descriptors that don't need to be passed to children to stop this misbehaviour.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>
>> v2:
>> - The existence of SOCK_CLOEXEC doesn't mean that accept4 exists. Added a check
>> for it in configure.
>>
>
> v3 even. Anthony, I hope this doesn't confuse your scripts?
>
No worries, I'll straighten it out.
> Kevin
>
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-12-02 14:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-02 11:24 [Qemu-devel] [PATCH v2] Don't leak file descriptors Kevin Wolf
2009-12-02 11:26 ` [Qemu-devel] " Kevin Wolf
2009-12-02 12:03 ` Scott Tsai
2009-12-02 12:58 ` Kevin Wolf
2009-12-02 14:04 ` Anthony Liguori
-- strict thread matches above, loose matches on Subject: below --
2009-11-16 15:47 [Qemu-devel] " Kevin Wolf
2009-12-01 15:55 ` Alexander Graf
2009-12-02 10:34 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).