Bug 1123007 - libguestfs 'direct' backend should close file descriptors before exec-ing qemu to avoid leaking !O_CLOEXEC fds
Summary: libguestfs 'direct' backend should close file descriptors before exec-ing qem...
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libguestfs
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Richard W.M. Jones
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 1123794 1123797
TreeView+ depends on / blocked
 
Reported: 2014-07-24 15:18 UTC by Richard W.M. Jones
Modified: 2014-07-31 13:05 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-07-31 13:05:03 UTC
Embargoed:


Attachments (Terms of Use)
leaky.pl (1.57 KB, text/plain)
2014-07-28 09:57 UTC, Richard W.M. Jones
no flags Details

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.


Note You need to log in before you can comment on or make changes to this bug.