From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Daniel P. Berrange" Subject: PATCH: Fix file descriptor leak in blktapctrl Date: Sat, 28 Jul 2007 01:46:56 +0100 Message-ID: <20070728004656.GC1140@redhat.com> Reply-To: "Daniel P. Berrange" Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="+HP7ph2BbKc20aGI" Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org --+HP7ph2BbKc20aGI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 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 -=| --+HP7ph2BbKc20aGI Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="xen-blktap-fd-leak.patch" 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 #include #include +#include #include #include #include @@ -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; --+HP7ph2BbKc20aGI Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --+HP7ph2BbKc20aGI--