Bug 1123007

Summary: libguestfs 'direct' backend should close file descriptors before exec-ing qemu to avoid leaking !O_CLOEXEC fds
Product: [Community] Virtualization Tools Reporter: Richard W.M. Jones <rjones>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED UPSTREAM QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: acathrow, hannsj_uhl, mbooth, ptoscano
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-07-31 13:05:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1123794, 1123797    
Attachments:
Description Flags
leaky.pl none

Description Richard W.M. Jones 2014-07-24 15:18:17 UTC 
Description of problem:

Python < 3.3 cannot reliably open file descriptors atomically
with O_CLOEXEC flag set.  As a result, there is the possibility
of a file descriptor being leaked into qemu.  See for example
this bug in Nova:
https://bugs.launchpad.net/nova/+bug/1313477

There may be other libraries and applications which have the
same problem and use libguestfs (libguestfs itself always uses
the non-racy calls to set O_CLOEXEC so our own file descriptors
are not affected).

In the direct backend, between fork + exec, we should therefore
close any file descriptors which are not intended to be passed
to qemu.

Note that libvirt does this already so we don't need to do it
in the libvirt backend.  It's unclear if this is required in
the UML backend.

Version-Release number of selected component (if applicable):

libguestfs 1.27.23

Steps to Reproduce:
1. See: https://bugs.launchpad.net/nova/+bug/1313477
Comment 1 Richard W.M. Jones 2014-07-25 13:08:12 UTC 
Patch posted:
https://www.redhat.com/archives/libguestfs/2014-July/msg00074.html
Comment 2 Richard W.M. Jones 2014-07-26 14:42:23 UTC 
Upstream in commit 115fcc34325f965ac3723683e4462fc667dcd254.
Comment 3 Richard W.M. Jones 2014-07-26 20:57:48 UTC 
Fix is included in libguestfs >= 1.26.6 & libguestfs >= 1.27.24.
Comment 4 Richard W.M. Jones 2014-07-28 09:57:24 UTC 
Created attachment 921744 [details]
leaky.pl

Test script which you can use to see if the bug has been fixed
in the currently installed version of libguestfs (note you will
need to install the Perl bindings -- perl-Sys-Guestfs).
Comment 5 Richard W.M. Jones 2014-07-29 21:27:21 UTC 
I *always* regret pushing patches without letting them live
in several releases of libguestfs ...

This patch causes a regression.  The child process should display
the qemu command line arguments, but it does not when this patch
is used.

Try running:

  LIBGUESTFS_BACKEND=direct libguestfs-test-tool

With this commit you don't see the full qemu command line.

I have reverted this patch in master and will look at fixing it later.
Comment 6 Richard W.M. Jones 2014-07-29 21:28:51 UTC 
Revert commit is 286f116691c8aad4ac8bf0d044e9f6ca8d5b3356.
Comment 7 Richard W.M. Jones 2014-07-30 13:28:51 UTC 
Patch v2 posted:

https://www.redhat.com/archives/libguestfs/2014-July/thread.html#00095
Comment 8 Richard W.M. Jones 2014-07-31 13:05:03 UTC 
v2 patch is upstream in libguestfs >= 1.27.25 and >= 1.26.7.