From 37132b4ea02a57056a426e04e77defb28e447ff9 Mon Sep 17 00:00:00 2001
From: Amar Tumballi <amarts@redhat.com>
Date: Tue, 24 Jul 2018 15:42:28 +0530
Subject: [PATCH 345/351] io-stats: allow only relative path for dumping
 io-stats info

It wouldn't make sense to allow iostats file to be written in
*any* directory. While the formating makes sure we try to append
io-stats-name for the file, so overwriting existing file is slim,
it makes sense to restrict dumping to one directory.

BUG: 1605086
Change-Id: Ie28b6a355de574d8d3855c8654d6756ef0389055
Signed-off-by: Amar Tumballi <amarts@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/145898
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
 tests/bugs/core/io-stats-1322825.t    | 12 ++++++------
 xlators/debug/io-stats/src/io-stats.c | 29 ++++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/tests/bugs/core/io-stats-1322825.t b/tests/bugs/core/io-stats-1322825.t
index d232ecb..53f2d04 100755
--- a/tests/bugs/core/io-stats-1322825.t
+++ b/tests/bugs/core/io-stats-1322825.t
@@ -23,7 +23,7 @@ TEST $CLI volume profile $V0 start
 TEST mkdir $M0/dir1
 
 # Generate the stat dump across the io-stat instances
-TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0
+TEST setfattr -n trusted.io-stats-dump -v io-stats-1322825 $M0
 
 # Check if $M0 is clean w.r.t xattr information
 # TODO: if there are better ways to check we really get no attr error, please
@@ -42,12 +42,12 @@ ret=$(echo $?)
 EXPECT 0 echo $ret
 
 # Check if we have 5 io-stat files in /tmp
-EXPECT 5 ls -1 /tmp/io-stats-1322825*
+EXPECT 5 ls -1 /var/run/gluster/io-stats-1322825*
 # Cleanup the 5 generated files
-rm -f /tmp/io-stats-1322825*
+rm -f /var/run/gluster/io-stats-1322825*
 
 # Rinse and repeat above for a directory
-TEST setfattr -n trusted.io-stats-dump -v /tmp/io-stats-1322825 $M0/dir1
+TEST setfattr -n trusted.io-stats-dump -v io-stats-1322825 $M0/dir1
 getfattr -n trusted.io-stats-dump $B0/${V0}1/dir1 2>&1 | grep -qi "no such attribute"
 ret=$(echo $?)
 EXPECT 0 echo $ret
@@ -61,7 +61,7 @@ getfattr -n trusted.io-stats-dump $B0/${V0}4/dir1 2>&1 | grep -qi "no such attri
 ret=$(echo $?)
 EXPECT 0 echo $ret
 
-EXPECT 5 ls -1 /tmp/io-stats-1322825*
-rm -f /tmp/io-stats-1322825*
+EXPECT 5 ls -1 /var/run/gluster/io-stats-1322825*
+rm -f /var/run/gluster/io-stats-1322825*
 
 cleanup;
diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
index f46474b..868890f 100644
--- a/xlators/debug/io-stats/src/io-stats.c
+++ b/xlators/debug/io-stats/src/io-stats.c
@@ -45,6 +45,8 @@
 #define DEFAULT_GRP_BUF_SZ 16384
 #define IOS_BLOCK_COUNT_SIZE 32
 
+#define IOS_STATS_DUMP_DIR DEFAULT_VAR_RUN_DIRECTORY
+
 typedef enum {
         IOS_STATS_TYPE_NONE,
         IOS_STATS_TYPE_OPEN,
@@ -3000,7 +3002,6 @@ io_stats_fsync (call_frame_t *frame, xlator_t *this,
         return 0;
 }
 
-
 int
 conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
 {
@@ -3016,6 +3017,7 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
         int                   pid, namelen;
         char                  dump_key[100];
         char                 *slash_ptr = NULL;
+        char                 *path_in_value = NULL;
 
         stub  = data;
         this  = stub->this;
@@ -3024,13 +3026,26 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
         name as well. This helps when there is more than a single io-stats
         instance in the graph, or the client and server processes are running
         on the same node */
-        /* hmmm... no check for this */
-        /* name format: <passed in path/filename>.<xlator name slashes to -> */
-        namelen = value->len + strlen (this->name) + 2; /* '.' and '\0' */
+        /* For the sanity of where the file should be located, we should make
+           sure file is written only inside RUNDIR (ie, /var/run/gluster) */
+        /* TODO: provide an option to dump it to different directory of
+           choice, based on options */
+        /* name format: /var/run/gluster/<passed in path/filename>.<xlator name slashes to -> */
+
+        path_in_value = data_to_str (value);
+
+        if (strstr (path_in_value, "../")) {
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s: no \"../\" allowed in path", path_in_value);
+                return -1;
+        }
+        namelen = (strlen (IOS_STATS_DUMP_DIR) + value->len +
+                   strlen (this->name) + 2);         /* '.' and '\0' */
+
         filename = alloca0 (namelen);
-        memcpy (filename, data_to_str (value), value->len);
-        memcpy (filename + value->len, ".", 1);
-        memcpy (filename + value->len + 1, this->name, strlen(this->name));
+
+        snprintf (filename, namelen, "%s%s.%s", IOS_STATS_DUMP_DIR,
+                  path_in_value, this->name);
 
         /* convert any slashes to '-' so that fopen works correctly */
         slash_ptr = strchr (filename + value->len + 1, '/');
-- 
1.8.3.1