Re: PATCH: Set close-on-exec flag for QEMU disks

["Daniel P. Berrange" <berrange@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2095 bytes --]

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" <berrange@redhat.com> 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 <berrange@redhat.com>

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  -=| 

[-- Attachment #2: xen-qemu-close-fds-2.patch --]
[-- Type: text/plain, Size: 1416 bytes --]

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);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel