Mailing List Archive

# HG changeset patch
# User kfraser@localhost.localdomain
# Date 1185955460 -3600
# Node ID 425c3d6f755791941233092e75edd8da08f5073c
# Parent 36caf6f8407214c054f685c84621cbe7c6420d73
Fix file descriptor leak in blktapctrl

The blktapctrl process is responsible for spawning individual tapdisk
processes. It does this using the 'system' method, but unfortunately
none of its file descriptors have the close-on-exec flag set. The
parent blktapctl process opens a couple of unix domain sockets
per-tapdisk it spawns. So the first tapdisk get 2 FDs leaked to it,
the second gets 4 FDs leaked to it, the 3rd gets 6 and so on. The use
of 'system' also unneccessarily invokes the shell.

Replace system with fork/execvp, and explicitly close all file handles
up to _SC_OPEN_MAX.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
tools/blktap/drivers/blktapctrl.c | 44 +++++++++++++++++++++++++++-----------
1 files changed, 32 insertions(+), 12 deletions(-)

diff -r 36caf6f84072 -r 425c3d6f7557 tools/blktap/drivers/blktapctrl.c
--- a/tools/blktap/drivers/blktapctrl.c Wed Aug 01 08:59:03 2007 +0100
+++ b/tools/blktap/drivers/blktapctrl.c Wed Aug 01 09:04:20 2007 +0100
@@ -42,6 +42,7 @@
#include <errno.h>
#include <sys/types.h>
#include <linux/types.h>
+#include <sys/wait.h>
#include <signal.h>
#include <fcntl.h>
#include <sys/poll.h>
@@ -472,11 +473,38 @@ static int read_msg(int fd, int msgtype,

}

+int launch_tapdisk(char *wrctldev, char *rdctldev)
+{
+ char *argv[] = { "tapdisk", wrctldev, rdctldev, NULL };
+ pid_t child;
+
+ if ((child = fork()) < 0)
+ return -1;
+
+ if (!child) {
+ int i;
+ for (i = 0 ; i < sysconf(_SC_OPEN_MAX) ; i++)
+ if (i != STDIN_FILENO &&
+ i != STDOUT_FILENO &&
+ i != STDERR_FILENO)
+ close(i);
+
+ execvp("tapdisk", argv);
+ _exit(1);
+ } else {
+ pid_t got;
+ do {
+ got = waitpid(child, NULL, 0);
+ } while (got != child);
+ }
+ return 0;
+}
+
int blktapctrl_new_blkif(blkif_t *blkif)
{
blkif_info_t *blk;
int major, minor, fd_read, fd_write, type, new;
- char *rdctldev, *wrctldev, *cmd, *ptr;
+ char *rdctldev, *wrctldev, *ptr;
image_t *image;
blkif_t *exist = NULL;
static uint16_t next_cookie = 0;
@@ -504,12 +532,6 @@ int blktapctrl_new_blkif(blkif_t *blkif)
free(rdctldev);
return -1;
}
- if (asprintf(&cmd, "tapdisk %s %s", wrctldev, rdctldev) == -1) {
- free(rdctldev);
- free(wrctldev);
- return -1;
- }
-
blkif->fds[READ] = open_ctrl_socket(rdctldev);
blkif->fds[WRITE] = open_ctrl_socket(wrctldev);

@@ -517,15 +539,14 @@ int blktapctrl_new_blkif(blkif_t *blkif)
goto fail;

/*launch the new process*/
- DPRINTF("Launching process, CMDLINE [%s]\n",cmd);
- if (system(cmd) == -1) {
- DPRINTF("Unable to fork, cmdline: [%s]\n",cmd);
+ DPRINTF("Launching process, CMDLINE [tapdisk %s %s]\n",wrctldev, rdctldev);
+ if (launch_tapdisk(wrctldev, rdctldev) == -1) {
+ DPRINTF("Unable to fork, cmdline: [tapdisk %s %s]\n",wrctldev, rdctldev);
return -1;
}

free(rdctldev);
free(wrctldev);
- free(cmd);
} else {
DPRINTF("Process exists!\n");
blkif->fds[READ] = exist->fds[READ];
@@ -605,7 +626,6 @@ int open_ctrl_socket(char *devname)
{
int ret;
int ipc_fd;
- char *cmd;
fd_set socks;
struct timeval timeout;


_______________________________________________
Xen-changelog mailing list
Xen-changelog@lists.xensource.com
http://lists.xensource.com/xen-changelog