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