From 71496826955cacac37abfd5fd017340a04988971 Mon Sep 17 00:00:00 2001
From: Krishnan Parthasarathi <kparthas@redhat.com>
Date: Wed, 20 Mar 2013 23:01:39 +0530
Subject: glusterfsd: Fixed fd leak due to use of tmpfile()

Change-Id: I3c2dc070ebe967100170e39f3545acacc6016d61
BUG: 924075
Signed-off-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-on: http://review.gluster.org/4703
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
---
 glusterfsd/src/glusterfsd-mgmt.c | 18 ++++++++++--
 libglusterfs/src/graph.y         | 60 +++++++++++++++++++++-------------------
 tests/bugs/bug-924075.t          | 23 +++++++++++++++
 3 files changed, 69 insertions(+), 32 deletions(-)
 create mode 100755 tests/bugs/bug-924075.t

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index 86412efaf..d28397a17 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -1408,14 +1408,21 @@ glusterfs_volfile_reconfigure (FILE *newvolfile_fp)
 {
         glusterfs_graph_t *oldvolfile_graph = NULL;
         glusterfs_graph_t *newvolfile_graph = NULL;
+        int                oldvolfile_fd    = -1;
         FILE              *oldvolfile_fp    = NULL;
         glusterfs_ctx_t   *ctx              = NULL;
+        char               template[PATH_MAX] = {0};
 
         int ret = -1;
 
-        oldvolfile_fp = tmpfile ();
-        if (!oldvolfile_fp)
+        strcpy (template, "/tmp/tmp.XXXXXX");
+        oldvolfile_fd = mkstemp (template);
+        oldvolfile_fp = fdopen (oldvolfile_fd, "w+b");
+        if (!oldvolfile_fp) {
+                gf_log ("glusterfsd-mgmt", GF_LOG_CRITICAL, "Failed to create "
+                        "temporary volfile");
                 goto out;
+        }
 
         if (!oldvollen) {
                 ret = 1; // Has to call INIT for the whole graph
@@ -1472,9 +1479,14 @@ glusterfs_volfile_reconfigure (FILE *newvolfile_fp)
 
         ret = 0;
 out:
-        if (oldvolfile_fp)
+        if (oldvolfile_fp) {
                 fclose (oldvolfile_fp);
 
+        } else if (-1 != oldvolfile_fd) {
+            close (oldvolfile_fd);
+
+        }
+
         return ret;
 }
 
diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y
index a640f2402..92267c4c7 100644
--- a/libglusterfs/src/graph.y
+++ b/libglusterfs/src/graph.y
@@ -553,52 +553,54 @@ glusterfs_graph_t *
 glusterfs_graph_construct (FILE *fp)
 {
         int                ret = 0;
+        int                tmp_fd = -1;
         glusterfs_graph_t *graph = NULL;
-	FILE              *tmp_file = NULL;
+        FILE              *tmp_file = NULL;
+        char               template[PATH_MAX] = {0};
 
         graph = glusterfs_graph_new ();
         if (!graph)
-                return NULL;
+                goto err;
 
-	tmp_file = tmpfile ();
+        strcpy (template, "/tmp/tmp.XXXXXX");
+        tmp_fd = mkstemp (template);
+        if (-1 == tmp_fd)
+                goto err;
 
-	if (tmp_file == NULL) {
-		gf_log ("parser", GF_LOG_ERROR,
-			"cannot create temporary file");
+        tmp_file = fdopen (tmp_fd, "w+b");
+        if (!tmp_file)
+                goto err;
 
-                glusterfs_graph_destroy (graph);
-		return NULL;
-	}
-
-	ret = preprocess (fp, tmp_file);
-	if (ret < 0) {
-		gf_log ("parser", GF_LOG_ERROR,
-			"parsing of backticks failed");
-
-                glusterfs_graph_destroy (graph);
-		fclose (tmp_file);
-		return NULL;
-	}
+        ret = preprocess (fp, tmp_file);
+        if (ret < 0) {
+                gf_log ("parser", GF_LOG_ERROR, "parsing of backticks failed");
+                goto err;
+        }
 
         yyin = tmp_file;
-
         construct = graph;
-
         ret = yyparse ();
-
         construct = NULL;
 
-	fclose (tmp_file);
-
         if (ret == 1) {
                 gf_log ("parser", GF_LOG_DEBUG,
-			"parsing of volfile failed, please review it "
-			"once more");
-
-                glusterfs_graph_destroy (graph);
-                return NULL;
+                        "parsing of volfile failed, please review it "
+                        "once more");
+                goto err;
         }
 
+        fclose (tmp_file);
         return graph;
+err:
+        if (tmp_file) {
+                fclose (tmp_file);
+        } else {
+                gf_log ("parser", GF_LOG_ERROR, "cannot create temporary file");
+                if (-1 != tmp_fd)
+                        close (tmp_fd);
+        }
+
+        glusterfs_graph_destroy (graph);
+        return NULL;
 }
 
diff --git a/tests/bugs/bug-924075.t b/tests/bugs/bug-924075.t
new file mode 100755
index 000000000..f4e03e33a
--- /dev/null
+++ b/tests/bugs/bug-924075.t
@@ -0,0 +1,23 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+#FIXME: there is another patch which moves the following function into
+#include.rc
+function process_leak_count ()
+{
+    local pid=$1;
+    return $(ls -lh /proc/$pid/fd | grep "(deleted)" | wc -l)
+}
+
+TEST glusterd;
+TEST $CLI volume create $V0 $H0:$B0/${V0}1;
+TEST $CLI volume start $V0;
+TEST glusterfs -s $H0 --volfile-id $V0 $M0;
+mount_pid=$(get_mount_process_pid $V0);
+TEST process_leak_count $mount_pid;
+
+cleanup;
-- 
cgit