<html><body>
<p><font size="2" face="sans-serif">Hi Richard,</font><br>
<br>
<tt><font size="2">I see </font></tt><tt><font size="2">this patch is included into libguestfs >= 1.26.6 & libguestfs >= 1.27.24. I feel that version is too high. Now RHEL 7.0 runs libguestfs-1.22.6, RHEL 6.6 alpha and 6.5 runs </font></tt><tt><font size="2">libguestfs-1.20.11. Is that possible to backport this patch, in order to make this problem fixed in RHEL 7.0, 6.6, and 6.5?</font></tt><br>
<br>
<font size="2" face="sans-serif">Thanks & Best Regards<br>
Qin Zhao (ÕÔÇÕ)<br>
China System and Technology Lab, IBM<br>
Tel: 86-10-82452339</font><br>
<br>
<img width="16" height="16" src="cid:1__=C7BBF7B0DFB0EBD98f9e8a93df938@cn.ibm.com" border="0" alt="Inactive hide details for "Richard W.M. Jones" ---2014/07/25 21:06:52---This refactors existing code to close file descriptors "><font size="2" color="#424282" face="sans-serif">"Richard W.M. Jones" ---2014/07/25 21:06:52---This refactors existing code to close file descriptors in the recovery process, and also adds code t</font><br>
<br>
<font size="1" color="#5F5F5F" face="sans-serif">From: </font><font size="1" face="sans-serif">"Richard W.M. Jones" <rjones@redhat.com></font><br>
<font size="1" color="#5F5F5F" face="sans-serif">To: </font><font size="1" face="sans-serif">libguestfs@redhat.com, </font><br>
<font size="1" color="#5F5F5F" face="sans-serif">Cc: </font><font size="1" face="sans-serif">ptoscano@redhat.com, berrange@redhat.com, Qin Zhao/China/IBM@IBMCN</font><br>
<font size="1" color="#5F5F5F" face="sans-serif">Date: </font><font size="1" face="sans-serif">2014/07/25 21:06</font><br>
<font size="1" color="#5F5F5F" face="sans-serif">Subject: </font><font size="1" face="sans-serif">[PATCH] launch: Close file descriptors after fork (RHBZ#1123007).</font><br>
<hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br>
<br>
<br>
<tt><font size="2">This refactors existing code to close file descriptors in the recovery<br>
process, and also adds code to close file descriptors between the<br>
fork() and exec() of QEMU or User-Mode Linux.<br>
<br>
The reason is to avoid leaking main process file descriptors where the<br>
main process (or other libraries in the main process) are not setting<br>
O_CLOEXEC at all or not setting it atomically. Python is a particular<br>
culprit.<br>
<br>
See also this OpenStack Nova bug report:<br>
</font></tt><tt><font size="2"><a href="https://bugs.launchpad.net/nova/+bug/1313477">https://bugs.launchpad.net/nova/+bug/1313477</a></font></tt><tt><font size="2"><br>
<br>
Thanks: Qin Zhao for identifying and characterizing the problem in Nova.<br>
---<br>
src/guestfs-internal-frontend.h | 16 +++++++++++++++-<br>
src/launch-direct.c | 17 +++++++++--------<br>
src/launch-uml.c | 13 +++++--------<br>
3 files changed, 29 insertions(+), 17 deletions(-)<br>
<br>
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h<br>
index 6bf0a94..3129018 100644<br>
--- a/src/guestfs-internal-frontend.h<br>
+++ b/src/guestfs-internal-frontend.h<br>
@@ -1,5 +1,5 @@<br>
/* libguestfs<br>
- * Copyright (C) 2013 Red Hat Inc.<br>
+ * Copyright (C) 2013-2014 Red Hat Inc.<br>
*<br>
* This library is free software; you can redistribute it and/or<br>
* modify it under the terms of the GNU Lesser General Public<br>
@@ -161,4 +161,18 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g, virDomain<br>
# define program_name "libguestfs"<br>
#endif<br>
<br>
+/* Close all file descriptors matching the condition. */<br>
+#define close_file_descriptors(cond) do { \<br>
+ int max_fd = sysconf (_SC_OPEN_MAX); \<br>
+ int fd; \<br>
+ if (max_fd == -1) \<br>
+ max_fd = 1024; \<br>
+ if (max_fd > 65536) \<br>
+ max_fd = 65536; /* bound the amount of work we do here */ \<br>
+ for (fd = 0; fd < max_fd; ++fd) { \<br>
+ if (cond) \<br>
+ close (fd); \<br>
+ } \<br>
+ } while (0)<br>
+<br>
#endif /* GUESTFS_INTERNAL_FRONTEND_H_ */<br>
diff --git a/src/launch-direct.c b/src/launch-direct.c<br>
index bb73d19..104809d 100644<br>
--- a/src/launch-direct.c<br>
+++ b/src/launch-direct.c<br>
@@ -717,6 +717,13 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)<br>
goto dup_failed;<br>
<br>
close (sv[1]);<br>
+<br>
+ /* Close any other file descriptors that we don't want to pass<br>
+ * to qemu. This prevents file descriptors which didn't have<br>
+ * O_CLOEXEC set properly from leaking into the subprocess. See<br>
+ * RHBZ#1123007.<br>
+ */<br>
+ close_file_descriptors (fd >= 2);<br>
}<br>
<br>
/* Dump the command line (after setting up stderr above). */<br>
@@ -747,7 +754,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)<br>
if (g->recovery_proc) {<br>
r = fork ();<br>
if (r == 0) {<br>
- int i, fd, max_fd;<br>
+ int i;<br>
struct sigaction sa;<br>
pid_t qemu_pid = data->pid;<br>
pid_t parent_pid = getppid ();<br>
@@ -767,13 +774,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)<br>
/* Close all other file descriptors. This ensures that we don't<br>
* hold open (eg) pipes from the parent process.<br>
*/<br>
- max_fd = sysconf (_SC_OPEN_MAX);<br>
- if (max_fd == -1)<br>
- max_fd = 1024;<br>
- if (max_fd > 65536)<br>
- max_fd = 65536; /* bound the amount of work we do here */<br>
- for (fd = 0; fd < max_fd; ++fd)<br>
- close (fd);<br>
+ close_file_descriptors (1);<br>
<br>
/* It would be nice to be able to put this in the same process<br>
* group as qemu (ie. setpgid (0, qemu_pid)). However this is<br>
diff --git a/src/launch-uml.c b/src/launch-uml.c<br>
index 2a6ddaf..88c684b 100644<br>
--- a/src/launch-uml.c<br>
+++ b/src/launch-uml.c<br>
@@ -333,6 +333,9 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)<br>
goto dup_failed;<br>
<br>
close (csv[1]);<br>
+<br>
+ /* RHBZ#1123007 */<br>
+ close_file_descriptors (fd >= 2 && fd != dsv[1]);<br>
}<br>
<br>
/* Dump the command line (after setting up stderr above). */<br>
@@ -360,7 +363,7 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)<br>
if (g->recovery_proc) {<br>
r = fork ();<br>
if (r == 0) {<br>
- int i, fd, max_fd;<br>
+ int i;<br>
struct sigaction sa;<br>
pid_t vmlinux_pid = data->pid;<br>
pid_t parent_pid = getppid ();<br>
@@ -380,13 +383,7 @@ launch_uml (guestfs_h *g, void *datav, const char *arg)<br>
/* Close all other file descriptors. This ensures that we don't<br>
* hold open (eg) pipes from the parent process.<br>
*/<br>
- max_fd = sysconf (_SC_OPEN_MAX);<br>
- if (max_fd == -1)<br>
- max_fd = 1024;<br>
- if (max_fd > 65536)<br>
- max_fd = 65536; /* bound the amount of work we do here */<br>
- for (fd = 0; fd < max_fd; ++fd)<br>
- close (fd);<br>
+ close_file_descriptors (1);<br>
<br>
/* It would be nice to be able to put this in the same process<br>
* group as vmlinux (ie. setpgid (0, vmlinux_pid)). However<br>
-- <br>
1.9.0<br>
<br>
</font></tt><br>
</body></html>