From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NFV4a-0006rv-HE for qemu-devel@nongnu.org; Tue, 01 Dec 2009 10:55:28 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NFV4V-0006n5-9X for qemu-devel@nongnu.org; Tue, 01 Dec 2009 10:55:27 -0500 Received: from [199.232.76.173] (port=59175 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NFV4V-0006mr-3K for qemu-devel@nongnu.org; Tue, 01 Dec 2009 10:55:23 -0500 Received: from cantor.suse.de ([195.135.220.2]:41412 helo=mx1.suse.de) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NFV4U-0000eL-4M for qemu-devel@nongnu.org; Tue, 01 Dec 2009 10:55:22 -0500 Message-ID: <4B153C60.2060605@suse.de> Date: Tue, 01 Dec 2009 16:55:12 +0100 From: Alexander Graf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2] Don't leak file descriptors References: <1258386420-23294-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1258386420-23294-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Anthony Liguori , qemu-devel@nongnu.org Kevin Wolf wrote: > We're leaking file descriptors to child processes. Set FD_CLOEXEC on fi= le > descriptors that don't need to be passed to children to stop this misbe= haviour. > > Signed-off-by: Kevin Wolf > --- > 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, co= nst char *filename, > s->open_flags |=3D O_DSYNC; > =20 > s->fd =3D -1; > - fd =3D open(filename, s->open_flags, 0644); > + fd =3D qemu_open(filename, s->open_flags, 0644); > if (fd < 0) { > ret =3D -errno; > if (ret =3D=3D -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 >=3D 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 > =20 > /* allow fast reuse */ > val =3D 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 =3D i; > =20 > s->vmfd =3D -1; > - s->fd =3D open("/dev/kvm", O_RDWR); > + s->fd =3D qemu_open("/dev/kvm", O_RDWR); > if (s->fd =3D=3D -1) { > fprintf(stderr, "Could not access KVM kernel module: %m\n"); > ret =3D -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 ch= ar *host_port, > s->state =3D MIG_STATE_ACTIVE; > s->mon_resume =3D NULL; > s->bandwidth_limit =3D bandwidth_limit; > - s->fd =3D socket(PF_INET, SOCK_STREAM, 0); > + s->fd =3D qemu_socket(PF_INET, SOCK_STREAM, 0); > if (s->fd =3D=3D -1) { > qemu_free(s); > return NULL; > @@ -139,7 +139,7 @@ static void tcp_accept_incoming_migration(void *opa= que) > int c, ret; > =20 > do { > - c =3D accept(s, (struct sockaddr *)&addr, &addrlen); > + c =3D qemu_accept(s, (struct sockaddr *)&addr, &addrlen); > } while (c =3D=3D -1 && socket_error() =3D=3D EINTR); > =20 > dprintf("accepted migration\n"); > @@ -186,7 +186,7 @@ int tcp_start_incoming_migration(const char *host_p= ort) > return -EINVAL; > } > =20 > - s =3D socket(PF_INET, SOCK_STREAM, 0); > + s =3D qemu_socket(PF_INET, SOCK_STREAM, 0); > if (s =3D=3D -1) > return -socket_error(); > =20 > 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 c= har *path, > s->state =3D MIG_STATE_ACTIVE; > s->mon_resume =3D NULL; > s->bandwidth_limit =3D bandwidth_limit; > - s->fd =3D socket(PF_UNIX, SOCK_STREAM, 0); > + s->fd =3D 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 *op= aque) > int c, ret; > =20 > do { > - c =3D accept(s, (struct sockaddr *)&addr, &addrlen); > + c =3D qemu_accept(s, (struct sockaddr *)&addr, &addrlen); > } while (c =3D=3D -1 && socket_error() =3D=3D EINTR); > =20 > dprintf("accepted migration\n"); > @@ -184,7 +184,7 @@ int unix_start_incoming_migration(const char *path) > =20 > dprintf("Attempting to start an incoming migration\n"); > =20 > - sock =3D socket(PF_UNIX, SOCK_STREAM, 0); > + sock =3D qemu_socket(PF_UNIX, SOCK_STREAM, 0); > if (sock < 0) { > fprintf(stderr, "Could not open unix socket: %s\n", strerror(e= rrno)); > 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 sockadd= r_in *mcastaddr) > return -1; > =20 > } > - fd =3D socket(PF_INET, SOCK_DGRAM, 0); > + fd =3D 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) > =20 > for(;;) { > len =3D sizeof(saddr); > - fd =3D accept(s->fd, (struct sockaddr *)&saddr, &len); > + fd =3D qemu_accept(s->fd, (struct sockaddr *)&saddr, &len); > if (fd < 0 && errno !=3D EINTR) { > return; > } else if (fd >=3D 0) { > @@ -1724,7 +1724,7 @@ static int net_socket_listen_init(VLANState *vlan= , > =20 > s =3D qemu_mallocz(sizeof(NetSocketListenState)); > =20 > - fd =3D socket(PF_INET, SOCK_STREAM, 0); > + fd =3D 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 *vla= n, > if (parse_host_port(&saddr, host_str) < 0) > return -1; > =20 > - fd =3D socket(PF_INET, SOCK_STREAM, 0); > + fd =3D 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; > =20 > - fd =3D open(filename, O_RDWR | O_CREAT, 0600); > + fd =3D qemu_open(filename, O_RDWR | O_CREAT, 0600); > if (fd =3D=3D -1) > return -1; > =20 > @@ -201,11 +201,113 @@ int inet_aton(const char *cp, struct in_addr *ia= ) > ia->s_addr =3D addr; > return 1; > } > + > +void qemu_set_cloexec(int fd) > +{ > +} > + > #else > + > void socket_set_nonblock(int fd) > { > int f; > f =3D fcntl(fd, F_GETFL); > fcntl(fd, F_SETFL, f | O_NONBLOCK); > } > + > +void qemu_set_cloexec(int fd) > +{ > + int f; > + f =3D 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 =3D 0; > + > + if (flags & O_CREAT) { > + va_list ap; > + > + va_start(ap, flags); > + mode =3D va_arg(ap, int); > + va_end(ap); > + } > + > +#ifdef O_CLOEXEC > + ret =3D open(name, flags | O_CLOEXEC, mode); > +#else > + ret =3D open(name, flags, mode); > + if (ret >=3D 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 =3D pipe2(pipefd, O_CLOEXEC); > +#else > + ret =3D pipe(pipefd); > + if (ret =3D=3D 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 =3D socket(domain, type | SOCK_CLOEXEC, protocol); > +#else > + ret =3D socket(domain, type, protocol); > + if (ret >=3D 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 =3D accept4(s, addr, addrlen, SOCK_CLOEXEC); > =20 On Anthony's staging tree: cc1: warnings being treated as errors osdep.c: In function =E2=80=98qemu_accept=E2=80=99: osdep.c:304: error: implicit declaration of function =E2=80=98accept4=E2=80= =99 Alex