From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: Re: PATCH: Set close-on-exec flag for QEMU disks Date: Tue, 6 Mar 2007 16:34:15 +0000 Message-ID: <20070306163415.GH29872@redhat.com> References: <45EC8A5F.3020308@us.ibm.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="H+4ONPRPur6+Ovig" Return-path: Content-Disposition: inline In-Reply-To: <45EC8A5F.3020308@us.ibm.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Anthony Liguori Cc: xen-devel@lists.xensource.com, Keir Fraser List-Id: xen-devel@lists.xenproject.org --H+4ONPRPur6+Ovig Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Mar 05, 2007 at 03:23:43PM -0600, Anthony Liguori wrote: > Keir Fraser wrote: > >On 2/3/07 21:40, "Daniel P. Berrange" wrote: > > > > > >>QEMU does not currently set the close-on-exec flag after opening its > >>virtual > >>disk images. This causes problems when it later runs the > >>/etc/xen/qemu-ifup > >>script because the file descriptors get propagated to networking commands > >>like brctl / ifconfig. The SELinux policy quite rightly does not allow the > >>networking scripts to access the virtual disk images, so these inherited > >>file descriptors for AVC denials to be logged. > >> > >>The attached patch modifies all the QEMU disk driver backends to make sure > >>the close-on-exec flag is turned on > >> > > > >It would be nicer to implement an open_cloexec() function in e.g., vl.c to > >do the open() and fcntl() in one go and in one place. > > > > There are few areas where scripts are executed. Why not just introduce > an exec() wrapper that closes file descriptors appropriately. Looking at the QEMU code in Xen there are only two places where exec is used, and one of those is for the vncviewer spawning which isn't upstream. One of them uses execlp() while the other uses execv(). So rather than doing a refactoring of code which will lead to even more divergance with upstream, the attached patch just fixes up those 2 locations to close all file handles upto sysconf(_SC_OPEN_MAX), with exception of STDIN/OUT/ERR and the TAP deevice handle. Signed-off-by: Daniel P. Berrange I've tested against Xen 3.0.4 in Fedora to validate neither the qemu-ifup or vncviewer processes receive any extraneous file handles. The attached patch applies cleanly to xen-unstable.hg too Regards, Dan. -- |=- 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 -=| --H+4ONPRPur6+Ovig Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-qemu-close-fds-2.patch" diff -r 3ac19fda0bc2 tools/ioemu/vl.c --- a/tools/ioemu/vl.c Fri Mar 02 12:11:52 2007 +0000 +++ b/tools/ioemu/vl.c Tue Mar 06 11:21:52 2007 -0500 @@ -3250,6 +3250,14 @@ static int net_tap_init(VLANState *vlan, pid = fork(); if (pid >= 0) { 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); + parg = args; *parg++ = (char *)setup_script; *parg++ = ifname; diff -r 3ac19fda0bc2 tools/ioemu/vnc.c --- a/tools/ioemu/vnc.c Fri Mar 02 12:11:52 2007 +0000 +++ b/tools/ioemu/vnc.c Tue Mar 06 11:21:52 2007 -0500 @@ -1445,7 +1445,7 @@ int vnc_display_init(DisplayState *ds, i int vnc_start_viewer(int port) { - int pid; + int pid, i, open_max; char s[16]; sprintf(s, ":%d", port); @@ -1456,6 +1456,12 @@ int vnc_start_viewer(int port) exit(1); case 0: /* child */ + open_max = sysconf (_SC_OPEN_MAX); + for (i = 0; i < open_max; i++) + if (i != STDIN_FILENO && + i != STDOUT_FILENO && + i != STDERR_FILENO) + close(i); execlp("vncviewer", "vncviewer", s, NULL); fprintf(stderr, "vncviewer execlp failed\n"); exit(1); --H+4ONPRPur6+Ovig 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 --H+4ONPRPur6+Ovig--