From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: [PATCH] Ensure FD_CLOEXEC is set on all XenD file handles Date: Tue, 15 Aug 2006 02:23:57 +0100 Message-ID: <20060815012357.GA28193@redhat.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="GvXjxJ+pjyke8COw" Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline While debugging an issue with libvirt I discovered a serious problem with XenD's management of file handles. Basically it does not set the close-on-exec flag on any[1] of the file handles it has open. This means that all XenD's file handles propagated to programs it spawns - eg, the network device setup scripts, qemu-dm and others those in turn spawn. The particular case that lead me to discover the problem was when I disabled HTTP port on XenD and wondered why port 8000 was still in LISTEN state - it turned out qemu-dm had a handle to the server socket. Aside from this it had access to about 10 open handles on /proc/xen/privcmd, the HTTP *client* connection which requested the domain creation[2], and any of the other server sockets XenD happened to have open. I'm attaching three prototype patches - I say prototype because there may be other (better) places to set the FD_CLOEXEC flag - I've just picked the place closest to the original socket()/accept() call. * xen-xend2-cloexec.patch - set the flag on the XMLRPC & HTTP server ports, both TCP & UNIX domain socket versions. Also ensure incoming clients have it set * xen-xc-cloexec.patch - set the flag on all handles to /proc/xen/privcmd * xen-xs2-cloexec.patch - set the flag on all connections to the xenstore daemon. The only file handle I've not yet set the flag on is the one to the log file /var/log/xend.log which (in my system) ends up on FD #9. To test which file handles were being left open I created a dummy script: $ cat > /root/fake-qemu-dm.sh <> /tmp/qemu-fds.log exec /usr/lib64/xen/bin/qemu-dm "$@" EOF $ And then in the domain's config file set 'device_model' to point to the file '/root/fake-qemu-dm.sh' BTW, the patches were prepared against the latest Xen userspace code in Fedora Core 6, test2 - this is trailing xen-unstable by a couple of weeks but I think they should still apply. If people agree with the approach taken in the patch I'll re-diff against xen-unstable before posting again. Regards, Dan. [1] Actually it turns out the relocation port does have the flag set [2] This explains an issue discovered with libvirt where it would hang forever after creating an HVM domain waiting for the server to close its end of the socket -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-xc-cloexec.patch" diff -ruN xen-unstable-10712/tools/libxc/xc_linux.c xen-unstable-10712-cloexec/tools/libxc/xc_linux.c --- xen-unstable-10712/tools/libxc/xc_linux.c 2006-07-21 13:31:22.000000000 -0400 +++ xen-unstable-10712-cloexec/tools/libxc/xc_linux.c 2006-08-14 20:24:15.000000000 -0400 @@ -13,13 +13,39 @@ #include #include +#include +#include int xc_interface_open(void) { + int flags, saved_errno; int fd = open("/proc/xen/privcmd", O_RDWR); - if ( fd == -1 ) + if ( fd == -1 ) { PERROR("Could not obtain handle on privileged command interface"); + return -1; + } + + /* Although we return the file handle as the 'xc handle' the API + does not specify / guarentee that this integer is in fact + a file handle. Thus we must take responsiblity to ensure + it doesn't propagate (ie leak) outside the process */ + if ((flags = fcntl(fd, F_GETFD)) < 0) { + PERROR("Could not get file handle flags"); + goto error; + } + flags |= FD_CLOEXEC; + if (fcntl(fd, F_SETFD, flags) < 0) { + PERROR("Could not set file handle flags"); + goto error; + } + return fd; + + error: + saved_errno = errno; + close(fd); + errno = saved_errno; + return -1; } int xc_interface_close(int xc_handle) --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-xend2-cloexec.patch" diff -ruN xen-unstable-10712/tools/python/xen/util/xmlrpclib2.py xen-unstable-10712-cloexec/tools/python/xen/util/xmlrpclib2.py --- xen-unstable-10712/tools/python/xen/util/xmlrpclib2.py 2006-07-21 13:31:22.000000000 -0400 +++ xen-unstable-10712-cloexec/tools/python/xen/util/xmlrpclib2.py 2006-08-14 21:00:33.000000000 -0400 @@ -22,6 +22,7 @@ import string import types +import fcntl from httplib import HTTPConnection, HTTP from xmlrpclib import Transport @@ -136,6 +137,17 @@ logRequests=1): SimpleXMLRPCServer.__init__(self, addr, requestHandler, logRequests) + flags = fcntl.fcntl(self.fileno(), fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(self.fileno(), fcntl.F_SETFD, flags) + + def get_request(self): + (client, addr) = SimpleXMLRPCServer.get_request(self) + flags = fcntl.fcntl(client.fileno(), fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(client.fileno(), fcntl.F_SETFD, flags) + return (client, addr) + def _marshaled_dispatch(self, data, dispatch_method = None): params, method = xmlrpclib.loads(data) try: diff -ruN xen-unstable-10712/tools/python/xen/web/httpserver.py xen-unstable-10712-cloexec/tools/python/xen/web/httpserver.py --- xen-unstable-10712/tools/python/xen/web/httpserver.py 2006-07-21 13:31:22.000000000 -0400 +++ xen-unstable-10712-cloexec/tools/python/xen/web/httpserver.py 2006-08-14 21:00:54.000000000 -0400 @@ -24,6 +24,7 @@ from urllib import quote, unquote import os import os.path +import fcntl from xen.xend import sxp from xen.xend.Args import ArgError @@ -294,6 +295,9 @@ def bind(self): self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + flags = fcntl.fcntl(self.socket.fileno(), fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(self.socket.fileno(), fcntl.F_SETFD, flags) self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) self.socket.bind((self.interface, self.port)) @@ -338,3 +342,6 @@ def bind(self): self.socket = unix.bind(self.path) + flags = fcntl.fcntl(self.socket.fileno(), fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(self.socket.fileno(), fcntl.F_SETFD, flags) --GvXjxJ+pjyke8COw Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-xs2-cloexec.patch" diff -ruN xen-unstable-10712/tools/xenstore/xs.c xen-unstable-10712-cloexec/tools/xenstore/xs.c --- xen-unstable-10712/tools/xenstore/xs.c 2006-08-14 20:31:20.000000000 -0400 +++ xen-unstable-10712-cloexec/tools/xenstore/xs.c 2006-08-14 20:32:26.000000000 -0400 @@ -101,23 +101,31 @@ static int get_socket(const char *connect_to) { struct sockaddr_un addr; - int sock, saved_errno; + int sock, saved_errno, flags; sock = socket(PF_UNIX, SOCK_STREAM, 0); if (sock < 0) return -1; + if ((flags = fcntl(sock, F_GETFD)) < 0) + goto error; + flags |= FD_CLOEXEC; + if (fcntl(sock, F_SETFD, flags) < 0) + goto error; + addr.sun_family = AF_UNIX; strcpy(addr.sun_path, connect_to); - if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) != 0) { - saved_errno = errno; - close(sock); - errno = saved_errno; - return -1; - } + if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) != 0) + goto error; return sock; + +error: + saved_errno = errno; + close(sock); + errno = saved_errno; + return -1; } static int get_dev(const char *connect_to) --GvXjxJ+pjyke8COw Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --GvXjxJ+pjyke8COw--