summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRichard W.M. Jones <rjones@redhat.com>2012-03-09 12:05:43 +0000
committerRichard W.M. Jones <rjones@redhat.com>2012-03-09 13:48:01 +0000
commitf1f045adf8d00549dd3efa3619e1162f9004b61e (patch)
tree78e974af7d863982207284f7d611b4db9901f73a
parent99702fe443383707b9d9c1b84570baf3bebf9253 (diff)
downloadlibguestfs-f1f045adf8d00549dd3efa3619e1162f9004b61e.tar.gz
libguestfs-f1f045adf8d00549dd3efa3619e1162f9004b61e.tar.xz
libguestfs-f1f045adf8d00549dd3efa3619e1162f9004b61e.zip
Close all file descriptors and remove all signal handlers in the recovery process.
If the parent process uses a pipe (or any fd, but pipes are a particular problem), then the recovery process would hold open the file descriptor(s) of the pipe, meaning that it could not be fully closed in the parent. Because the recovery process doesn't use exec(2), this wasn't avoidable even using FD_CLOEXEC. Avoid this by closing all file descriptors when starting the recovery process. After discussion with Dan Berrange, he points out that it's also a good idea to set signal handlers to the default after forking, so that any signal handlers set up in the parent don't affect the child.
-rw-r--r--src/launch.c25
1 files changed, 25 insertions, 0 deletions
diff --git a/src/launch.c b/src/launch.c
index 1dc23f40..1b496f69 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -844,9 +844,34 @@ launch_appliance (guestfs_h *g)
if (g->recovery_proc) {
r = fork ();
if (r == 0) {
+ int i, fd, max_fd;
+ struct sigaction sa;
pid_t qemu_pid = g->pid;
pid_t parent_pid = getppid ();
+ /* Remove all signal handlers. See the justification here:
+ * https://www.redhat.com/archives/libvir-list/2008-August/msg00303.html
+ * We don't mask signal handlers yet, so this isn't completely
+ * race-free, but better than not doing it at all.
+ */
+ memset (&sa, 0, sizeof sa);
+ sa.sa_handler = SIG_DFL;
+ sa.sa_flags = 0;
+ sigemptyset (&sa.sa_mask);
+ for (i = 1; i < NSIG; ++i)
+ sigaction (i, &sa, NULL);
+
+ /* Close all other file descriptors. This ensures that we don't
+ * hold open (eg) pipes from the parent process.
+ */
+ max_fd = sysconf (_SC_OPEN_MAX);
+ if (max_fd == -1)
+ max_fd = 1024;
+ if (max_fd > 65536)
+ max_fd = 65536; /* bound the amount of work we do here */
+ for (fd = 0; fd < max_fd; ++fd)
+ close (fd);
+
/* It would be nice to be able to put this in the same process
* group as qemu (ie. setpgid (0, qemu_pid)). However this is
* not possible because we don't have any guarantee here that