diff options
author | Richard W.M. Jones <rjones@redhat.com> | 2012-03-09 12:05:43 +0000 |
---|---|---|
committer | Richard W.M. Jones <rjones@redhat.com> | 2012-03-09 13:48:01 +0000 |
commit | f1f045adf8d00549dd3efa3619e1162f9004b61e (patch) | |
tree | 78e974af7d863982207284f7d611b4db9901f73a | |
parent | 99702fe443383707b9d9c1b84570baf3bebf9253 (diff) | |
download | libguestfs-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.c | 25 |
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 |