From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NABFk-0005MD-L6
	for qemu-devel@nongnu.org; Mon, 16 Nov 2009 18:45:00 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1NABFe-0005LF-VQ
	for qemu-devel@nongnu.org; Mon, 16 Nov 2009 18:44:59 -0500
Received: from [199.232.76.173] (port=46700 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NABFe-0005L9-If
	for qemu-devel@nongnu.org; Mon, 16 Nov 2009 18:44:54 -0500
Received: from mail2.shareable.org ([80.68.89.115]:59114)
	by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32)
	(Exim 4.60) (envelope-from <jamie@shareable.org>) id 1NABFe-0005nk-3y
	for qemu-devel@nongnu.org; Mon, 16 Nov 2009 18:44:54 -0500
Date: Mon, 16 Nov 2009 23:44:51 +0000
From: Jamie Lokier <jamie@shareable.org>
Subject: Re: [Qemu-devel] [PATCH] Don't leak file descriptors
Message-ID: <20091116234451.GK12063@shareable.org>
References: <1258125436-23759-1-git-send-email-kwolf@redhat.com>
	<4B01DF9B.6010407@codemonkey.ws>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4B01DF9B.6010407@codemonkey.ws>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org

Anthony Liguori wrote:
> 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>
> >  
> 
>    pid = fork();
>    if (pid == 0) {
>        int open_max = sysconf(_SC_OPEN_MAX), i;
> 
>        for (i = 0; i < open_max; i++) {
>            if (i != STDIN_FILENO &&
>                i != STDOUT_FILENO &&
>                i != STDERR_FILENO &&
>                i != fd) {
>                close(i);
>            }
> 
> Handles this in a less invasive way.  I think the only problem we have 
> today is that we use popen() for exec: migration.  The solution to that 
> though should be to convert popen to a proper fork/exec() with a pipe.
> 
> I'd prefer to introduce a single fork/exec helper that behaved properly 
> instead of having to deal with cloexec everywhere.

The above can be a bit slow when sysconf(_SC_OPEN_MAX) == 131072,
which you get if running qemu from some web servers or some user
environments set up to run web servers...  But it's not _that_ slow on
a modern machine on Linux - 10^7 closes per second has been measured.
Still a bit slow if it's INT_MAX :-)

A scalable method on Linux is readdir(/proc/self/fd).  (I'm not sure
if readdir returns everything reliably if you close while reading, so
just reading to get the largest open fd value, then closing all fds up
to that value is what I do).

Or just copy the closefrom() implementation from openssh/sudo.

Interestingly, that says "We avoid checking resource limits since it
is possible to open a file descriptor and then drop the rlimit such
that it is below the open fd..." but then uses _SC_OPEN_MAX, which I
think on Glibc checks the resource limits...

-- Jamie