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>

1 files changed, 32 insertions, 12 deletions

diff --git a/tools/blktap/drivers/blktapctrl.c b/tools/blktap/drivers/blktapctrl.c
index 6613768654..7045880668 100644
--- a/tools/blktap/drivers/blktapctrl.c
+++ b/tools/blktap/drivers/blktapctrl.c
@@ -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, void *ptr)
 
 }
 
+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;