PATCH: Fix file descriptor leak in blktapctrl

["Daniel P. Berrange" <berrange@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

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. So the attached patch replaces system with fork/execvp, and explicitly
closes all file handles upto _SC_OPEN_MAX

  Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

[-- Attachment #2: xen-blktap-fd-leak.patch --]
[-- Type: text/plain, Size: 2340 bytes --]

diff -r 1f348e70a5af tools/blktap/drivers/blktapctrl.c
--- a/tools/blktap/drivers/blktapctrl.c	Tue Jul 10 11:10:38 2007 +0100
+++ b/tools/blktap/drivers/blktapctrl.c	Fri Jul 27 20:30:31 2007 -0400
@@ -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;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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