[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors |
Date: |
Tue, 01 Dec 2009 16:55:12 +0100 |
User-agent: |
Thunderbird 2.0.0.23 (X11/20090817) |
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 <address@hidden>
> ---
> 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
- Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors,
Alexander Graf <=